Re: [SeaBIOS] [PATCH 0/5] Simplify SeaVGABIOS headers

[Kevin O'Connor]

On Fri, Aug 05, 2016 at 12:23:15PM -0400, Kevin O'Connor wrote:
> This series reorganizes the vgabios headers so that there is just one
> header for each C file, except for vgautil.h which is used for C files
> that only need simple function and variable extern definitions.

FYI, I committed this series.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v3 3/3] support booting with more than 255 CPUs

[Kevin O'Connor]

On Fri, Aug 05, 2016 at 12:47:29PM +0200, Igor Mammedov wrote:
> SDM[*1] says that if there are CPUs with APIC ID
> greater than 254, BIOS is to pass control to OS
> in x2APIC mode. Use the fact that QEMU passes in
> "etc/max-cpus" max possible "APIC ID + 1" to
> detect need for x2APIC mode. Also instead of
> CMOS_BIOS_SMP_COUNT which is limited to 256 CPUs
> use a new rom file "etc/boot-cpus" that QEMU
> supporting more than 256 CPUs will provide.
> 
> *1) SDM: Volume 3: EXTENDED XAPIC (X2APIC):
>  Initialization by System Software
> 
> Signed-off-by: Igor Mammedov 
> ---
> v2:
>   * merge handle_x2apic() into apic_id_init()
> ---
>  src/x86.h|  1 +
>  src/fw/smp.c | 26 +-
>  2 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/src/x86.h b/src/x86.h
> index 53378e9..a770e6f 100644
> --- a/src/x86.h
> +++ b/src/x86.h
> @@ -68,6 +68,7 @@ static inline void wbinvd(void)
>  #define CPUID_MSR (1 << 5)
>  #define CPUID_APIC (1 << 9)
>  #define CPUID_MTRR (1 << 12)
> +#define CPUID_X2APIC (1 << 21)
>  static inline void __cpuid(u32 index, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx)
>  {
>  asm("cpuid"
> diff --git a/src/fw/smp.c b/src/fw/smp.c
> index 2c5670c..352f52e 100644
> --- a/src/fw/smp.c
> +++ b/src/fw/smp.c
> @@ -19,6 +19,9 @@
>  #define APIC_LINT1   ((u8*)BUILD_APIC_ADDR + 0x360)
>  
>  #define APIC_ENABLED 0x0100
> +#define MSR_IA32_APIC_BASE 0x01B
> +#define MSR_LOCAL_APIC_ID 0x802
> +#define MSR_IA32_APICBASE_EXTD (1ULL << 10) /* Enable x2APIC mode */
>  
>  static struct { u32 index; u64 val; } smp_msr[32];
>  static u32 smp_msr_count;
> @@ -46,6 +49,7 @@ smp_write_msrs(void)
>  }
>  
>  u32 MaxCountCPUs;
> +static u16 boot_cpus_count;
>  static u32 CountCPUs;
>  // 256 bits for the found APIC IDs
>  static u32 FoundAPICIDs[256/32];
> @@ -58,13 +62,24 @@ int apic_id_is_present(u8 apic_id)
>  static int
>  apic_id_init(void)
>  {
> +u32 apic_id;
>  CountCPUs++;
>  
> -// Track found apic id for use in legacy internal bios tables
>  u32 eax, ebx, ecx, cpuid_features;
>  cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> -u8 apic_id = ebx>>24;
> -FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> +apic_id = ebx>>24;
> +if (MaxCountCPUs < 256) { // xAPIC mode
> +// Track found apic id for use in legacy internal bios tables
> +FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> +} else if (ecx & CPUID_X2APIC) {
> +// switch to x2APIC mode
> +u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
> +wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
> +apic_id = rdmsr(MSR_LOCAL_APIC_ID);
> +} else {
> +// x2APIC is masked by CPUID
> +apic_id = -1;
> +}
>  
>  return apic_id;
>  }
> @@ -132,8 +147,7 @@ smp_scan(void)
>  apic_id_init();
>  
>  // Wait for other CPUs to process the SIPI.
> -u8 cmos_smp_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
> -while (cmos_smp_count != CountCPUs)
> +while (boot_cpus_count != CountCPUs)
>  asm volatile(
>  // Release lock and allow other processors to use the stack.
>  "  movl %%esp, %1\n"
> @@ -164,6 +178,8 @@ smp_setup(void)
>  if (MaxCountCPUs < cmos_smp_count)
>  MaxCountCPUs = cmos_smp_count;
>  
> +boot_cpus_count = romfile_loadint("etc/boot-cpus", cmos_smp_count);
> +

Wouldn't boot_cpus_count then also need to be updated in smp_resume()?
If the user hotplugs a new cpu between system start and prior to an s3
resume event, then the new could should be handled during that resume.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v3 2/3] smp: refactor present CPU APIC ID detection and counting

[Kevin O'Connor]

On Fri, Aug 05, 2016 at 12:47:28PM +0200, Igor Mammedov wrote:
> From: Kevin O'Connor 
> 
> Signed-off-by: "Kevin O'Connor" 
> Signed-off-by: Igor Mammedov 
> ---
> v2:
>   * s/count_cpu/apic_id_init/
>   * call apic_id_init() after sending SIPI,
> it will be needed for switching BSP into x2APIC mode
> ---
>  src/fw/smp.c | 32 +++-
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/src/fw/smp.c b/src/fw/smp.c
> index 719d51d..2c5670c 100644
> --- a/src/fw/smp.c
> +++ b/src/fw/smp.c
> @@ -55,24 +55,32 @@ int apic_id_is_present(u8 apic_id)
>  return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32)));
>  }
>  
> +static int
> +apic_id_init(void)
> +{
> +CountCPUs++;
> +
> +// Track found apic id for use in legacy internal bios tables
> +u32 eax, ebx, ecx, cpuid_features;
> +cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> +u8 apic_id = ebx>>24;
> +FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> +
> +return apic_id;
> +}
> +
>  void VISIBLE32FLAT
>  handle_smp(void)
>  {
>  if (!CONFIG_QEMU)
>  return;
>  
> -// Detect apic_id
> -u32 eax, ebx, ecx, cpuid_features;
> -cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> -u8 apic_id = ebx>>24;
> +// Track this CPU and detect the apic_id
> +int apic_id = apic_id_init();
>  dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
>  
>  smp_write_msrs();
>  
> -// Set bit on FoundAPICIDs
> -FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> -
> -CountCPUs++;
>  }
>  
>  // Atomic lock for shared stack across processors.
> @@ -93,11 +101,6 @@ smp_scan(void)
>  return;
>  }
>  
> -// mark the BSP initial APIC ID as found, too:
> -u8 apic_id = ebx>>24;
> -FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> -CountCPUs = 1;

Now that scan_smp() is called from the resume path, we do have to
initialize CountCPUs here.  Probably best to not move the updating of
CountCPUs into apic_id_init().

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 5/5] vgautil: Move definitions from cbvga.h and clext.h to vgautil.h

[Kevin O'Connor]

These files only need to export simple function definitions - move
them to vgautil.h.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/cbvga.c   |  1 -
 vgasrc/cbvga.h   | 20 
 vgasrc/clext.c   |  1 -
 vgasrc/clext.h   | 20 
 vgasrc/vgabios.c |  1 -
 vgasrc/vgahw.h   |  2 --
 vgasrc/vgautil.h | 31 ++-
 7 files changed, 30 insertions(+), 46 deletions(-)
 delete mode 100644 vgasrc/cbvga.h
 delete mode 100644 vgasrc/clext.h

diff --git a/vgasrc/cbvga.c b/vgasrc/cbvga.c
index 9d0adef..b8216a9 100644
--- a/vgasrc/cbvga.c
+++ b/vgasrc/cbvga.c
@@ -5,7 +5,6 @@
 // This file may be distributed under the terms of the GNU LGPLv3 license.
 
 #include "biosvar.h" // GET_BDA
-#include "cbvga.h" // cbvga_setup
 #include "output.h" // dprintf
 #include "stdvga.h" // SEG_CTEXT
 #include "string.h" // memset16_far
diff --git a/vgasrc/cbvga.h b/vgasrc/cbvga.h
deleted file mode 100644
index fb892c8..000
--- a/vgasrc/cbvga.h
+++ /dev/null
@@ -1,20 +0,0 @@
-#ifndef __CBVGA_H
-#define __CBVGA_H
-
-#include "types.h" // u16
-
-struct vgamode_s *cbvga_find_mode(int mode);
-void cbvga_list_modes(u16 seg, u16 *dest, u16 *last);
-int cbvga_get_window(struct vgamode_s *vmode_g, int window);
-int cbvga_set_window(struct vgamode_s *vmode_g, int window, int val);
-int cbvga_get_linelength(struct vgamode_s *vmode_g);
-int cbvga_set_linelength(struct vgamode_s *vmode_g, int val);
-int cbvga_get_displaystart(struct vgamode_s *vmode_g);
-int cbvga_set_displaystart(struct vgamode_s *vmode_g, int val);
-int cbvga_get_dacformat(struct vgamode_s *vmode_g);
-int cbvga_set_dacformat(struct vgamode_s *vmode_g, int val);
-int cbvga_save_restore(int cmd, u16 seg, void *data);
-int cbvga_set_mode(struct vgamode_s *vmode_g, int flags);
-int cbvga_setup(void);
-
-#endif // cbvga.h
diff --git a/vgasrc/clext.c b/vgasrc/clext.c
index 45b5de3..da8b790 100644
--- a/vgasrc/clext.c
+++ b/vgasrc/clext.c
@@ -7,7 +7,6 @@
 
 #include "biosvar.h" // GET_GLOBAL
 #include "bregs.h" // struct bregs
-#include "clext.h" // clext_setup
 #include "hw/pci.h" // pci_config_readl
 #include "hw/pci_regs.h" // PCI_BASE_ADDRESS_0
 #include "output.h" // dprintf
diff --git a/vgasrc/clext.h b/vgasrc/clext.h
deleted file mode 100644
index cf47a5b..000
--- a/vgasrc/clext.h
+++ /dev/null
@@ -1,20 +0,0 @@
-#ifndef __CLEXT_H
-#define __CLEXT_H
-
-#include "types.h" // u16
-
-struct vgamode_s *clext_find_mode(int mode);
-void clext_list_modes(u16 seg, u16 *dest, u16 *last);
-int clext_get_window(struct vgamode_s *vmode_g, int window);
-int clext_set_window(struct vgamode_s *vmode_g, int window, int val);
-int clext_get_linelength(struct vgamode_s *vmode_g);
-int clext_set_linelength(struct vgamode_s *vmode_g, int val);
-int clext_get_displaystart(struct vgamode_s *vmode_g);
-int clext_set_displaystart(struct vgamode_s *vmode_g, int val);
-int clext_save_restore(int cmd, u16 seg, void *data);
-int clext_set_mode(struct vgamode_s *vmode_g, int flags);
-struct bregs;
-void clext_1012(struct bregs *regs);
-int clext_setup(void);
-
-#endif // clext.h
diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index db4f868..3b9694c 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -7,7 +7,6 @@
 
 #include "biosvar.h" // GET_BDA
 #include "bregs.h" // struct bregs
-#include "clext.h" // clext_1012
 #include "config.h" // CONFIG_*
 #include "output.h" // dprintf
 #include "std/vbe.h" // VBE_RETURN_STATUS_FAILED
diff --git a/vgasrc/vgahw.h b/vgasrc/vgahw.h
index 3d9ae39..dab2b4d 100644
--- a/vgasrc/vgahw.h
+++ b/vgasrc/vgahw.h
@@ -4,8 +4,6 @@
 #include "types.h" // u8
 #include "config.h" // CONFIG_*
 
-#include "cbvga.h" // cbvga_setup
-#include "clext.h" // clext_set_mode
 #include "bochsvga.h" // bochsvga_set_mode
 #include "stdvga.h" // stdvga_set_mode
 #include "geodevga.h" // geodevga_setup
diff --git a/vgasrc/vgautil.h b/vgasrc/vgautil.h
index 90fa54d..79c16be 100644
--- a/vgasrc/vgautil.h
+++ b/vgasrc/vgautil.h
@@ -4,6 +4,36 @@
 
 #include "types.h" // u8
 
+// cbvga.c
+struct vgamode_s *cbvga_find_mode(int mode);
+void cbvga_list_modes(u16 seg, u16 *dest, u16 *last);
+int cbvga_get_window(struct vgamode_s *vmode_g, int window);
+int cbvga_set_window(struct vgamode_s *vmode_g, int window, int val);
+int cbvga_get_linelength(struct vgamode_s *vmode_g);
+int cbvga_set_linelength(struct vgamode_s *vmode_g, int val);
+int cbvga_get_displaystart(struct vgamode_s *vmode_g);
+int cbvga_set_displaystart(struct vgamode_s *vmode_g, int val);
+int cbvga_get_dacformat(struct vgamode_s *vmode_g);
+int cbvga_set_dacformat(struct vgamode_s *vmode_g, int val);
+int cbvga_save_restore(int cmd, u16 seg, void *data);
+int cbvga_set_mode(struct vgamode_s *vmode_g, in

[SeaBIOS] [PATCH 2/5] vgainit: Move video param setup to stdvga_build_video_param()

[Kevin O'Connor]

Move the full video_param_table[] setup (including the updating of the
BDA) to stdvga_build_video_param().

Signed-off-by: Kevin O'Connor 
---
 vgasrc/stdvgamodes.c | 10 ++
 vgasrc/vgabios.c |  1 +
 vgasrc/vgabios.h |  9 +++--
 vgasrc/vgainit.c |  8 
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/vgasrc/stdvgamodes.c b/vgasrc/stdvgamodes.c
index c553514..f61d52f 100644
--- a/vgasrc/stdvgamodes.c
+++ b/vgasrc/stdvgamodes.c
@@ -7,6 +7,7 @@
 
 #include "biosvar.h" // GET_GLOBAL
 #include "output.h" // warn_internalerror
+#include "std/vga.h" // struct video_param_s
 #include "stdvga.h" // stdvga_find_mode
 #include "string.h" // memcpy_far
 #include "vgabios.h" // video_param_table
@@ -348,9 +349,18 @@ stdvga_list_modes(u16 seg, u16 *dest, u16 *last)
 SET_FARVAR(seg, *dest, 0x);
 }
 
+static struct video_save_pointer_s video_save_pointer_table VAR16;
+
+static struct video_param_s video_param_table[29] VAR16;
+
 void
 stdvga_build_video_param(void)
 {
+SET_BDA(video_savetable
+, SEGOFF(get_global_seg(), (u32)&video_save_pointer_table));
+SET_VGA(video_save_pointer_table.videoparam
+, SEGOFF(get_global_seg(), (u32)video_param_table));
+
 static u8 parammodes[] VAR16 = {
 0, 0, 0, 0, 0x04, 0x05, 0x06, 0x07,
 0, 0, 0, 0, 0, 0x0d, 0x0e, 0,
diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index 44ce312..b980da5 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -11,6 +11,7 @@
 #include "config.h" // CONFIG_*
 #include "output.h" // dprintf
 #include "std/vbe.h" // VBE_RETURN_STATUS_FAILED
+#include "std/vga.h" // struct video_func_static
 #include "stdvga.h" // stdvga_set_cursor_shape
 #include "string.h" // memset_far
 #include "vgabios.h" // calc_page_size
diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index 0e988bd..2176ec6 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -2,9 +2,8 @@
 #define __VGABIOS_H
 
 #include "config.h" // CONFIG_VGA_EMULATE_TEXT
+#include "farptr.h" // GET_FARVAR
 #include "types.h" // u8
-#include "farptr.h" // struct segoff_s
-#include "std/vga.h" // struct video_param_s
 
 // Save/Restore flags
 #define SR_HARDWARE   0x0001
@@ -81,12 +80,10 @@ extern u8 vgafont14alt[];
 extern u8 vgafont16alt[];
 
 // vgainit.c
-extern struct video_save_pointer_s video_save_pointer_table;
-extern struct video_param_s video_param_table[29];
-
-// vgabios.c
 extern int VgaBDF;
 extern int HaveRunInit;
+
+// vgabios.c
 #define SET_VGA(var, val) SET_FARVAR(get_global_seg(), (var), (val))
 int vga_bpp(struct vgamode_s *vmode_g);
 u16 calc_page_size(u8 memmodel, u16 width, u16 height);
diff --git a/vgasrc/vgainit.c b/vgasrc/vgainit.c
index 6249e66..f003026 100644
--- a/vgasrc/vgainit.c
+++ b/vgasrc/vgainit.c
@@ -18,10 +18,6 @@
 #include "vgabios.h" // video_save_pointer_table
 #include "vgahw.h" // vgahw_setup
 
-struct video_save_pointer_s video_save_pointer_table VAR16;
-
-struct video_param_s video_param_table[29] VAR16;
-
 // Type of emulator platform - for dprintf with certain compile options.
 int PlatformRunningOn VAR16;
 
@@ -132,8 +128,6 @@ init_bios_area(void)
 SET_BDA(modeset_ctl, 0x51);
 
 SET_BDA(dcc_index, CONFIG_VGA_STDVGA_PORTS ? 0x08 : 0xff);
-SET_BDA(video_savetable
-, SEGOFF(get_global_seg(), (u32)&video_save_pointer_table));
 
 // FIXME
 SET_BDA(video_msr, 0x00); // Unavailable on vanilla vga, but...
@@ -171,8 +165,6 @@ vga_post(struct bregs *regs)
 
 init_bios_area();
 
-SET_VGA(video_save_pointer_table.videoparam
-, SEGOFF(get_global_seg(), (u32)video_param_table));
 if (CONFIG_VGA_STDVGA_PORTS)
 stdvga_build_video_param();
 
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 4/5] vgautil: Move generic definitions from stdvga.h to vgautil.h

[Kevin O'Connor]

Don't use stdvga.h for function definitions of code in stdvgamodes.c
and stdvgaio.c.  Move them to vgautil.h.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/bochsvga.c |  2 +-
 vgasrc/stdvga.c   |  1 +
 vgasrc/stdvga.h   | 32 +---
 vgasrc/stdvgaio.c |  3 ++-
 vgasrc/vgahw.h|  1 +
 vgasrc/vgautil.h  | 31 +++
 6 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/vgasrc/bochsvga.c b/vgasrc/bochsvga.c
index 5658031..ec5d101 100644
--- a/vgasrc/bochsvga.c
+++ b/vgasrc/bochsvga.c
@@ -13,7 +13,7 @@
 #include "hw/pci_regs.h" // PCI_BASE_ADDRESS_0
 #include "output.h" // dprintf
 #include "std/vbe.h" // VBE_CAPABILITY_8BIT_DAC
-#include "stdvga.h" // VGAREG_SEQU_ADDRESS
+#include "stdvga.h" // stdvga_get_linelength
 #include "vgabios.h" // SET_VGA
 #include "vgautil.h" // VBE_total_memory
 #include "x86.h" // outw
diff --git a/vgasrc/stdvga.c b/vgasrc/stdvga.c
index 00a0fc5..886deca 100644
--- a/vgasrc/stdvga.c
+++ b/vgasrc/stdvga.c
@@ -10,6 +10,7 @@
 #include "stdvga.h" // stdvga_setup
 #include "string.h" // memset_far
 #include "vgabios.h" // struct vgamode_s
+#include "vgautil.h" // stdvga_attr_write
 #include "x86.h" // outb
 
 
diff --git a/vgasrc/stdvga.h b/vgasrc/stdvga.h
index 39753b4..4184c60 100644
--- a/vgasrc/stdvga.h
+++ b/vgasrc/stdvga.h
@@ -44,37 +44,6 @@
 #define SEG_CTEXT 0xB800
 #define SEG_MTEXT 0xB000
 
-// stdvgamodes.c
-struct vgamode_s *stdvga_find_mode(int mode);
-void stdvga_list_modes(u16 seg, u16 *dest, u16 *last);
-void stdvga_build_video_param(void);
-void stdvga_override_crtc(int mode, u8 *crtc);
-int stdvga_set_mode(struct vgamode_s *vmode_g, int flags);
-void stdvga_set_packed_palette(void);
-
-// stdvgaio.c
-u8 stdvga_pelmask_read(void);
-void stdvga_pelmask_write(u8 val);
-u8 stdvga_misc_read(void);
-void stdvga_misc_write(u8 value);
-void stdvga_misc_mask(u8 off, u8 on);
-u8 stdvga_sequ_read(u8 index);
-void stdvga_sequ_write(u8 index, u8 value);
-void stdvga_sequ_mask(u8 index, u8 off, u8 on);
-u8 stdvga_grdc_read(u8 index);
-void stdvga_grdc_write(u8 index, u8 value);
-void stdvga_grdc_mask(u8 index, u8 off, u8 on);
-u8 stdvga_crtc_read(u16 crtc_addr, u8 index);
-void stdvga_crtc_write(u16 crtc_addr, u8 index, u8 value);
-void stdvga_crtc_mask(u16 crtc_addr, u8 index, u8 off, u8 on);
-u8 stdvga_attr_read(u8 index);
-void stdvga_attr_write(u8 index, u8 value);
-void stdvga_attr_mask(u8 index, u8 off, u8 on);
-u8 stdvga_attrindex_read(void);
-void stdvga_attrindex_write(u8 value);
-void stdvga_dac_read(u16 seg, u8 *data_far, u8 start, int count);
-void stdvga_dac_write(u16 seg, u8 *data_far, u8 start, int count);
-
 // stdvga.c
 void stdvga_set_border_color(u8 color);
 void stdvga_set_overscan_border_color(u8 color);
@@ -91,6 +60,7 @@ void stdvga_planar4_plane(int plane);
 void stdvga_load_font(u16 seg, void *src_far, u16 count
   , u16 start, u8 destflags, u8 fontsize);
 u16 stdvga_get_crtc(void);
+struct vgamode_s;
 int stdvga_vram_ratio(struct vgamode_s *vmode_g);
 void stdvga_set_cursor_shape(u16 cursor_type);
 void stdvga_set_cursor_pos(int address);
diff --git a/vgasrc/stdvgaio.c b/vgasrc/stdvgaio.c
index d6138c2..77fcecd 100644
--- a/vgasrc/stdvgaio.c
+++ b/vgasrc/stdvgaio.c
@@ -5,7 +5,8 @@
 // This file may be distributed under the terms of the GNU LGPLv3 license.
 
 #include "farptr.h" // GET_FARVAR
-#include "stdvga.h" // stdvga_pelmask_read
+#include "stdvga.h" // VGAREG_PEL_MASK
+#include "vgautil.h" // stdvga_pelmask_read
 #include "x86.h" // inb
 
 u8
diff --git a/vgasrc/vgahw.h b/vgasrc/vgahw.h
index 39f818a..3d9ae39 100644
--- a/vgasrc/vgahw.h
+++ b/vgasrc/vgahw.h
@@ -9,6 +9,7 @@
 #include "bochsvga.h" // bochsvga_set_mode
 #include "stdvga.h" // stdvga_set_mode
 #include "geodevga.h" // geodevga_setup
+#include "vgautil.h" // stdvga_list_modes
 
 static inline struct vgamode_s *vgahw_find_mode(int mode) {
 if (CONFIG_VGA_CIRRUS)
diff --git a/vgasrc/vgautil.h b/vgasrc/vgautil.h
index 9e4debb..90fa54d 100644
--- a/vgasrc/vgautil.h
+++ b/vgasrc/vgautil.h
@@ -4,6 +4,37 @@
 
 #include "types.h" // u8
 
+// stdvgaio.c
+u8 stdvga_pelmask_read(void);
+void stdvga_pelmask_write(u8 val);
+u8 stdvga_misc_read(void);
+void stdvga_misc_write(u8 value);
+void stdvga_misc_mask(u8 off, u8 on);
+u8 stdvga_sequ_read(u8 index);
+void stdvga_sequ_write(u8 index, u8 value);
+void stdvga_sequ_mask(u8 index, u8 off, u8 on);
+u8 stdvga_grdc_read(u8 index);
+void stdvga_grdc_write(u8 index, u8 value);
+void stdvga_grdc_mask(u8 index, u8 off, u8 on);
+u8 stdvga_crtc_read(u16 crtc_addr, u8 index);
+void stdvga_crtc_write(u16 crtc_addr, u8 index, u8 value);
+void stdvga_crtc_mask(u16 crtc_addr, u8 index, u8 off, u8 on);
+u8 stdvga_attr_read(u8 index);

[SeaBIOS] [PATCH 1/5] vgafb: Move header definitions from vgabios.h to new file vgafb.h

[Kevin O'Connor]

Signed-off-by: Kevin O'Connor 
---
 vgasrc/cbvga.c|  1 +
 vgasrc/swcursor.c |  3 ++-
 vgasrc/vgabios.c  |  1 +
 vgasrc/vgabios.h  | 36 
 vgasrc/vgafb.c|  1 +
 vgasrc/vgafb.h| 42 ++
 6 files changed, 47 insertions(+), 37 deletions(-)
 create mode 100644 vgasrc/vgafb.h

diff --git a/vgasrc/cbvga.c b/vgasrc/cbvga.c
index 1cfb9d3..b4d7d36 100644
--- a/vgasrc/cbvga.c
+++ b/vgasrc/cbvga.c
@@ -11,6 +11,7 @@
 #include "string.h" // memset16_far
 #include "util.h" // find_cb_table
 #include "vgabios.h" // VGAREG_*
+#include "vgafb.h" // handle_gfx_op
 
 static int CBmode VAR16;
 static struct vgamode_s CBmodeinfo VAR16;
diff --git a/vgasrc/swcursor.c b/vgasrc/swcursor.c
index 83f4822..b7b3250 100644
--- a/vgasrc/swcursor.c
+++ b/vgasrc/swcursor.c
@@ -6,7 +6,8 @@
 
 #include "biosvar.h" // GET_BDA
 #include "bregs.h" // struct bregs
-#include "vgabios.h" // handle_gfx_op
+#include "vgabios.h" // get_cursor_pos
+#include "vgafb.h" // handle_gfx_op
 
 // Draw/undraw a cursor on the framebuffer by xor'ing the cursor cell
 static void
diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index 4e897c4..44ce312 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -14,6 +14,7 @@
 #include "stdvga.h" // stdvga_set_cursor_shape
 #include "string.h" // memset_far
 #include "vgabios.h" // calc_page_size
+#include "vgafb.h" // vgafb_write_char
 #include "vgahw.h" // vgahw_set_mode
 
 
diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index ffbb729..0e988bd 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -43,25 +43,6 @@ struct vgamode_s {
 u16 sstart;
 };
 
-// Graphics pixel operations.
-struct gfx_op {
-struct vgamode_s *vmode_g;
-u32 linelength;
-u32 displaystart;
-
-u8 op;
-u16 x, y;
-
-u8 pixels[8];
-u16 xlen, ylen;
-u16 srcy;
-};
-
-#define GO_READ8   1
-#define GO_WRITE8  2
-#define GO_MEMSET  3
-#define GO_MEMMOVE 4
-
 // Custom internal storage in BDA (don't change here without also
 // updating vgaentry.S)
 #define VGA_CUSTOM_BDA 0xb9
@@ -107,12 +88,6 @@ extern struct video_param_s video_param_table[29];
 extern int VgaBDF;
 extern int HaveRunInit;
 #define SET_VGA(var, val) SET_FARVAR(get_global_seg(), (var), (val))
-struct carattr {
-u8 car, attr, use_attr, pad;
-};
-struct cursorpos {
-u8 x, y, page, pad;
-};
 int vga_bpp(struct vgamode_s *vmode_g);
 u16 calc_page_size(u8 memmodel, u16 width, u16 height);
 u16 get_cursor_shape(void);
@@ -122,17 +97,6 @@ struct vgamode_s *get_current_mode(void);
 int vga_set_mode(int mode, int flags);
 extern struct video_func_static static_functionality;
 
-// vgafb.c
-void init_gfx_op(struct gfx_op *op, struct vgamode_s *vmode_g);
-void handle_gfx_op(struct gfx_op *op);
-void *text_address(struct cursorpos cp);
-void vgafb_scroll(struct cursorpos win, struct cursorpos winsize
-  , int lines, struct carattr ca);
-void vgafb_write_char(struct cursorpos cp, struct carattr ca);
-struct carattr vgafb_read_char(struct cursorpos cp);
-void vgafb_write_pixel(u8 color, u16 x, u16 y);
-u8 vgafb_read_pixel(u16 x, u16 y);
-
 // swcursor.c
 struct bregs;
 void swcursor_pre_handle10(struct bregs *regs);
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 57ecc9b..04d543e 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -11,6 +11,7 @@
 #include "stdvga.h" // stdvga_planar4_plane
 #include "string.h" // memset_far
 #include "vgabios.h" // vgafb_scroll
+#include "vgafb.h" // vgafb_write_char
 #include "vgahw.h" // vgahw_get_linelength
 
 static inline void
diff --git a/vgasrc/vgafb.h b/vgasrc/vgafb.h
new file mode 100644
index 000..ccdc703
--- /dev/null
+++ b/vgasrc/vgafb.h
@@ -0,0 +1,42 @@
+#ifndef __VGAFB_H
+#define __VGAFB_H
+
+// Graphics pixel operations.
+struct gfx_op {
+struct vgamode_s *vmode_g;
+u32 linelength;
+u32 displaystart;
+
+u8 op;
+u16 x, y;
+
+u8 pixels[8];
+u16 xlen, ylen;
+u16 srcy;
+};
+
+#define GO_READ8   1
+#define GO_WRITE8  2
+#define GO_MEMSET  3
+#define GO_MEMMOVE 4
+
+struct cursorpos {
+u8 x, y, page, pad;
+};
+
+struct carattr {
+u8 car, attr, use_attr, pad;
+};
+
+// vgafb.c
+void init_gfx_op(struct gfx_op *op, struct vgamode_s *vmode_g);
+void handle_gfx_op(struct gfx_op *op);
+void *text_address(struct cursorpos cp);
+void vgafb_scroll(struct cursorpos win, struct cursorpos winsize
+  , int lines, struct carattr ca);
+void vgafb_write_char(struct cursorpos cp, struct carattr ca);
+struct carattr vgafb_read_char(struct cursorpos cp);
+void vgafb_write_pixel(u8 color, u16 x, u16 y);
+u8 vgafb_read_pixel(u16 x, u16 y);
+
+#endif // vgafb.h
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 0/5] Simplify SeaVGABIOS headers

[Kevin O'Connor]

This series reorganizes the vgabios headers so that there is just one
header for each C file, except for vgautil.h which is used for C files
that only need simple function and variable extern definitions.

-Kevin


Kevin O'Connor (5):
  vgafb: Move header definitions from vgabios.h to new file vgafb.h
  vgainit: Move video param setup to stdvga_build_video_param()
  vgautil: Add new header file with misc function and variable
definitions
  vgautil: Move generic definitions from stdvga.h to vgautil.h
  vgautil: Move definitions from cbvga.h and clext.h to vgautil.h

 vgasrc/bochsvga.c|  5 +--
 vgasrc/cbvga.c   |  5 +--
 vgasrc/cbvga.h   | 20 
 vgasrc/clext.c   |  4 +--
 vgasrc/clext.h   | 20 
 vgasrc/geodevga.c|  3 +-
 vgasrc/stdvga.c  |  1 +
 vgasrc/stdvga.h  | 32 +--
 vgasrc/stdvgaio.c|  3 +-
 vgasrc/stdvgamodes.c | 13 +++-
 vgasrc/swcursor.c|  4 ++-
 vgasrc/vbe.c |  8 -
 vgasrc/vgabios.c |  4 ++-
 vgasrc/vgabios.h | 72 +++--
 vgasrc/vgafb.c   |  4 ++-
 vgasrc/vgafb.h   | 42 
 vgasrc/vgafonts.c|  2 +-
 vgasrc/vgahw.h   |  3 +-
 vgasrc/vgainit.c | 11 ++-
 vgasrc/vgautil.h | 90 
 20 files changed, 182 insertions(+), 164 deletions(-)
 delete mode 100644 vgasrc/cbvga.h
 delete mode 100644 vgasrc/clext.h
 create mode 100644 vgasrc/vgafb.h
 create mode 100644 vgasrc/vgautil.h

-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 3/5] vgautil: Add new header file with misc function and variable definitions

[Kevin O'Connor]

Move the generic function and variable definitions from vgabios.h to a
new file vgautil.h.  This reduces the size and complexity of
vgabios.h.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/bochsvga.c|  3 ++-
 vgasrc/cbvga.c   |  3 ++-
 vgasrc/clext.c   |  3 ++-
 vgasrc/geodevga.c|  3 ++-
 vgasrc/stdvgamodes.c |  3 ++-
 vgasrc/swcursor.c|  1 +
 vgasrc/vbe.c |  8 +++-
 vgasrc/vgabios.c |  1 +
 vgasrc/vgabios.h | 31 +++
 vgasrc/vgafb.c   |  3 ++-
 vgasrc/vgafonts.c|  2 +-
 vgasrc/vgainit.c |  3 ++-
 vgasrc/vgautil.h | 30 ++
 13 files changed, 57 insertions(+), 37 deletions(-)
 create mode 100644 vgasrc/vgautil.h

diff --git a/vgasrc/bochsvga.c b/vgasrc/bochsvga.c
index aa82fc5..5658031 100644
--- a/vgasrc/bochsvga.c
+++ b/vgasrc/bochsvga.c
@@ -14,7 +14,8 @@
 #include "output.h" // dprintf
 #include "std/vbe.h" // VBE_CAPABILITY_8BIT_DAC
 #include "stdvga.h" // VGAREG_SEQU_ADDRESS
-#include "vgabios.h" // struct vbe_modeinfo
+#include "vgabios.h" // SET_VGA
+#include "vgautil.h" // VBE_total_memory
 #include "x86.h" // outw
 
 
diff --git a/vgasrc/cbvga.c b/vgasrc/cbvga.c
index b4d7d36..9d0adef 100644
--- a/vgasrc/cbvga.c
+++ b/vgasrc/cbvga.c
@@ -10,8 +10,9 @@
 #include "stdvga.h" // SEG_CTEXT
 #include "string.h" // memset16_far
 #include "util.h" // find_cb_table
-#include "vgabios.h" // VGAREG_*
+#include "vgabios.h" // SET_VGA
 #include "vgafb.h" // handle_gfx_op
+#include "vgautil.h" // VBE_total_memory
 
 static int CBmode VAR16;
 static struct vgamode_s CBmodeinfo VAR16;
diff --git a/vgasrc/clext.c b/vgasrc/clext.c
index fc5b42f..45b5de3 100644
--- a/vgasrc/clext.c
+++ b/vgasrc/clext.c
@@ -13,7 +13,8 @@
 #include "output.h" // dprintf
 #include "stdvga.h" // VGAREG_SEQU_ADDRESS
 #include "string.h" // memset16_far
-#include "vgabios.h" // VBE_VENDOR_STRING
+#include "vgabios.h" // SET_VGA
+#include "vgautil.h" // VBE_total_memory
 
 
 /
diff --git a/vgasrc/geodevga.c b/vgasrc/geodevga.c
index f8f61c3..a5a58cd 100644
--- a/vgasrc/geodevga.c
+++ b/vgasrc/geodevga.c
@@ -13,7 +13,8 @@
 #include "hw/pci_regs.h" // PCI_BASE_ADDRESS_0
 #include "output.h" // dprintf
 #include "stdvga.h" // stdvga_crtc_write
-#include "vgabios.h" // VGAREG_*
+#include "vgabios.h" // SET_VGA
+#include "vgautil.h" // VBE_total_memory
 
 
 /
diff --git a/vgasrc/stdvgamodes.c b/vgasrc/stdvgamodes.c
index f61d52f..173dd4f 100644
--- a/vgasrc/stdvgamodes.c
+++ b/vgasrc/stdvgamodes.c
@@ -10,7 +10,8 @@
 #include "std/vga.h" // struct video_param_s
 #include "stdvga.h" // stdvga_find_mode
 #include "string.h" // memcpy_far
-#include "vgabios.h" // video_param_table
+#include "vgabios.h" // SET_VGA
+#include "vgautil.h" // vgafont16
 
 
 /
diff --git a/vgasrc/swcursor.c b/vgasrc/swcursor.c
index b7b3250..f2212d5 100644
--- a/vgasrc/swcursor.c
+++ b/vgasrc/swcursor.c
@@ -8,6 +8,7 @@
 #include "bregs.h" // struct bregs
 #include "vgabios.h" // get_cursor_pos
 #include "vgafb.h" // handle_gfx_op
+#include "vgautil.h" // swcursor_check_event
 
 // Draw/undraw a cursor on the framebuffer by xor'ing the cursor cell
 static void
diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
index af3d0cc..facad19 100644
--- a/vgasrc/vbe.c
+++ b/vgasrc/vbe.c
@@ -12,8 +12,14 @@
 #include "output.h" // dprintf
 #include "std/vbe.h" // struct vbe_info
 #include "string.h" // memset_far
-#include "vgabios.h" // handle_104f
+#include "vgabios.h" // get_current_mode
 #include "vgahw.h" // vgahw_set_mode
+#include "vgautil.h" // handle_104f
+
+#define VBE_OEM_STRING "SeaBIOS VBE(C) 2011"
+#define VBE_VENDOR_STRING "SeaBIOS Developers"
+#define VBE_PRODUCT_STRING "SeaBIOS VBE Adapter"
+#define VBE_REVISION_STRING "Rev. 1"
 
 u32 VBE_total_memory VAR16 = 256 * 1024;
 u32 VBE_capabilities VAR16;
diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index b980da5..db4f868 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -17,6 +17,7 @@
 #include "vgabios.h" // calc_page_size
 #include "vgafb.h" // vgafb_write_char
 #include "vgahw.h" // vgahw_set_mode
+#include "vgautil.h" // swcursor_pre_handle10
 
 
 /
diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index 2176ec6..3d5bbfe 100644
--- a/v

[SeaBIOS] [PATCH 1/2] swcursor: Move swcursor code from vgafb.c to new file swcursor.c

[Kevin O'Connor]

Signed-off-by: Kevin O'Connor 
---
 Makefile  |  2 +-
 vgasrc/swcursor.c | 65 +++
 vgasrc/vgabios.h  |  2 ++
 vgasrc/vgafb.c| 57 
 4 files changed, 68 insertions(+), 58 deletions(-)
 create mode 100644 vgasrc/swcursor.c

diff --git a/Makefile b/Makefile
index 4930b3a..41bfcf4 100644
--- a/Makefile
+++ b/Makefile
@@ -207,7 +207,7 @@ $(OUT)bios.bin.elf: $(OUT)rom.o $(OUT)bios.bin.prep
 
 # VGA src files
 SRCVGA=src/output.c src/string.c src/hw/pci.c src/hw/serialio.c \
-vgasrc/vgainit.c vgasrc/vgabios.c vgasrc/vgafb.c \
+vgasrc/vgainit.c vgasrc/vgabios.c vgasrc/vgafb.c vgasrc/swcursor.c \
 vgasrc/vgafonts.c vgasrc/vbe.c \
 vgasrc/stdvga.c vgasrc/stdvgamodes.c vgasrc/stdvgaio.c \
 vgasrc/clext.c vgasrc/bochsvga.c vgasrc/geodevga.c \
diff --git a/vgasrc/swcursor.c b/vgasrc/swcursor.c
new file mode 100644
index 000..35f857a
--- /dev/null
+++ b/vgasrc/swcursor.c
@@ -0,0 +1,65 @@
+// Virtual software based cursor support
+//
+// Copyright (C) 2014-2016  Kevin O'Connor 
+//
+// This file may be distributed under the terms of the GNU LGPLv3 license.
+
+#include "biosvar.h" // GET_BDA
+#include "vgabios.h" // handle_gfx_op
+
+// Draw/undraw a cursor on the framebuffer by xor'ing the cursor cell
+static void
+gfx_set_swcursor(struct vgamode_s *vmode_g, int enable, struct cursorpos cp)
+{
+u16 cursor_type = get_cursor_shape();
+u8 start = cursor_type >> 8, end = cursor_type & 0xff;
+struct gfx_op op;
+init_gfx_op(&op, vmode_g);
+op.x = cp.x * 8;
+int cheight = GET_BDA(char_height);
+op.y = cp.y * cheight + start;
+
+int i;
+for (i = start; i < cheight && i <= end; i++, op.y++) {
+op.op = GO_READ8;
+handle_gfx_op(&op);
+int j;
+for (j = 0; j < 8; j++)
+op.pixels[j] ^= 0x07;
+op.op = GO_WRITE8;
+handle_gfx_op(&op);
+}
+}
+
+// Draw/undraw a cursor on the screen
+void
+vgafb_set_swcursor(int enable)
+{
+if (!vga_emulate_text())
+return;
+u8 flags = GET_BDA_EXT(flags);
+if (!!(flags & BF_SWCURSOR) == enable)
+// Already in requested mode.
+return;
+struct vgamode_s *vmode_g = get_current_mode();
+if (!vmode_g)
+return;
+struct cursorpos cp = get_cursor_pos(GET_BDA(video_page));
+if (cp.x >= GET_BDA(video_cols) || cp.y > GET_BDA(video_rows)
+|| GET_BDA(cursor_type) >= 0x2000)
+// Cursor not visible
+return;
+
+SET_BDA_EXT(flags, (flags & ~BF_SWCURSOR) | (enable ? BF_SWCURSOR : 0));
+
+if (GET_GLOBAL(vmode_g->memmodel) != MM_TEXT) {
+gfx_set_swcursor(vmode_g, enable, cp);
+return;
+}
+
+// In text mode, swap foreground and background attributes for cursor
+void *dest_far = text_address(cp) + 1;
+u8 attr = GET_FARVAR(GET_GLOBAL(vmode_g->sstart), *(u8*)dest_far);
+attr = (attr >> 4) | (attr << 4);
+SET_FARVAR(GET_GLOBAL(vmode_g->sstart), *(u8*)dest_far, attr);
+}
diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index 9fbfb14..9764020 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -132,6 +132,8 @@ void vgafb_write_char(struct cursorpos cp, struct carattr 
ca);
 struct carattr vgafb_read_char(struct cursorpos cp);
 void vgafb_write_pixel(u8 color, u16 x, u16 y);
 u8 vgafb_read_pixel(u16 x, u16 y);
+
+// swcursor.c
 void vgafb_set_swcursor(int enable);
 
 // vbe.c
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index bd1c8dd..46a1ab8 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -497,30 +497,6 @@ fail:
 return (struct carattr){0, 0, 0};
 }
 
-// Draw/undraw a cursor on the framebuffer by xor'ing the cursor cell
-void
-gfx_set_swcursor(struct vgamode_s *vmode_g, int enable, struct cursorpos cp)
-{
-u16 cursor_type = get_cursor_shape();
-u8 start = cursor_type >> 8, end = cursor_type & 0xff;
-struct gfx_op op;
-init_gfx_op(&op, vmode_g);
-op.x = cp.x * 8;
-int cheight = GET_BDA(char_height);
-op.y = cp.y * cheight + start;
-
-int i;
-for (i = start; i < cheight && i <= end; i++, op.y++) {
-op.op = GO_READ8;
-handle_gfx_op(&op);
-int j;
-for (j = 0; j < 8; j++)
-op.pixels[j] ^= 0x07;
-op.op = GO_WRITE8;
-handle_gfx_op(&op);
-}
-}
-
 // Set the pixel at the given position.
 void
 vgafb_write_pixel(u8 color, u16 x, u16 y)
@@ -686,36 +662,3 @@ vgafb_read_char(struct cursorpos cp)
 u16 v = GET_FARVAR(GET_GLOBAL(vmode_g->sstart), *dest_far);
 return (struct carattr){v, v>>8, 0};
 }
-
-// Draw/undraw a cursor on the screen
-void
-vgafb_set_swcursor(int enable)
-{
-if (!vga_emulate_text())
-return;
-u8 flags = GET_BDA_EXT(flags);
-if (!!(flags & BF_SWCURSOR) == enable)

[SeaBIOS] [PATCH 2/2] swcursor: Concentrate swcursor logic in swcursor.c

[Kevin O'Connor]

The software cursor code is not frequently used (only the coreboot
framebuffer vga code uses it).  Move its logic out of the main code
and into swcursor.c.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/swcursor.c | 37 +
 vgasrc/vgabios.c  |  8 ++--
 vgasrc/vgabios.h  |  5 +++--
 vgasrc/vgafb.c|  5 -
 vgasrc/vgainit.c  |  4 +---
 5 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/vgasrc/swcursor.c b/vgasrc/swcursor.c
index 35f857a..83f4822 100644
--- a/vgasrc/swcursor.c
+++ b/vgasrc/swcursor.c
@@ -5,6 +5,7 @@
 // This file may be distributed under the terms of the GNU LGPLv3 license.
 
 #include "biosvar.h" // GET_BDA
+#include "bregs.h" // struct bregs
 #include "vgabios.h" // handle_gfx_op
 
 // Draw/undraw a cursor on the framebuffer by xor'ing the cursor cell
@@ -32,11 +33,9 @@ gfx_set_swcursor(struct vgamode_s *vmode_g, int enable, 
struct cursorpos cp)
 }
 
 // Draw/undraw a cursor on the screen
-void
-vgafb_set_swcursor(int enable)
+static void
+set_swcursor(int enable)
 {
-if (!vga_emulate_text())
-return;
 u8 flags = GET_BDA_EXT(flags);
 if (!!(flags & BF_SWCURSOR) == enable)
 // Already in requested mode.
@@ -63,3 +62,33 @@ vgafb_set_swcursor(int enable)
 attr = (attr >> 4) | (attr << 4);
 SET_FARVAR(GET_GLOBAL(vmode_g->sstart), *(u8*)dest_far, attr);
 }
+
+// Disable virtual cursor if a vgabios call accesses the framebuffer
+void
+swcursor_pre_handle10(struct bregs *regs)
+{
+if (!vga_emulate_text())
+return;
+switch (regs->ah) {
+case 0x4f:
+if (!CONFIG_VGA_VBE || regs->al != 0x02)
+break;
+// NO BREAK
+case 0x00 ... 0x02:
+case 0x05 ... 0x0e:
+case 0x13:
+set_swcursor(0);
+break;
+default:
+break;
+}
+}
+
+// Called by periodic (18.2hz) timer
+void
+swcursor_check_event(void)
+{
+if (!vga_emulate_text())
+return;
+set_swcursor(GET_BDA(timer_counter) % 18 < 9);
+}
diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index 2c8cc79..4e897c4 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -74,7 +74,6 @@ get_cursor_shape(void)
 static void
 set_cursor_shape(u16 cursor_type)
 {
-vgafb_set_swcursor(0);
 SET_BDA(cursor_type, cursor_type);
 if (CONFIG_VGA_STDVGA_PORTS)
 stdvga_set_cursor_shape(get_cursor_shape());
@@ -89,7 +88,6 @@ set_cursor_pos(struct cursorpos cp)
 
 if (cp.page == GET_BDA(video_page)) {
 // Update cursor in hardware
-vgafb_set_swcursor(0);
 if (CONFIG_VGA_STDVGA_PORTS)
 stdvga_set_cursor_pos((int)text_address(cp));
 }
@@ -118,8 +116,6 @@ set_active_page(u8 page)
 if (!vmode_g)
 return;
 
-vgafb_set_swcursor(0);
-
 // Calculate memory address of start of page
 struct cursorpos cp = {0, 0, page};
 int address = (int)text_address(cp);
@@ -268,8 +264,6 @@ vga_set_mode(int mode, int flags)
 if (!vmode_g)
 return VBE_RETURN_STATUS_FAILED;
 
-vgafb_set_swcursor(0);
-
 int ret = vgahw_set_mode(vmode_g, flags);
 if (ret)
 return ret;
@@ -1103,6 +1097,8 @@ void VISIBLE16
 handle_10(struct bregs *regs)
 {
 debug_enter(regs, DEBUG_VGA_10);
+swcursor_pre_handle10(regs);
+
 switch (regs->ah) {
 case 0x00: handle_1000(regs); break;
 case 0x01: handle_1001(regs); break;
diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index 9764020..ffbb729 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -134,7 +134,9 @@ void vgafb_write_pixel(u8 color, u16 x, u16 y);
 u8 vgafb_read_pixel(u16 x, u16 y);
 
 // swcursor.c
-void vgafb_set_swcursor(int enable);
+struct bregs;
+void swcursor_pre_handle10(struct bregs *regs);
+void swcursor_check_event(void);
 
 // vbe.c
 extern u32 VBE_total_memory;
@@ -145,7 +147,6 @@ extern u16 VBE_win_granularity;
 #define VBE_VENDOR_STRING "SeaBIOS Developers"
 #define VBE_PRODUCT_STRING "SeaBIOS VBE Adapter"
 #define VBE_REVISION_STRING "Rev. 1"
-struct bregs;
 void handle_104f(struct bregs *regs);
 
 #endif // vgabios.h
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 46a1ab8..57ecc9b 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -504,7 +504,6 @@ vgafb_write_pixel(u8 color, u16 x, u16 y)
 struct vgamode_s *vmode_g = get_current_mode();
 if (!vmode_g)
 return;
-vgafb_set_swcursor(0);
 
 struct gfx_op op;
 init_gfx_op(&op, vmode_g);
@@ -529,7 +528,6 @@ vgafb_read_pixel(u16 x, u16 y)
 struct vgamode_s *vmode_g = get_current_mode();
 if (!vmode_g)
 return 0;
-vgafb_set_swcursor(0);
 
 struct gfx_op op;
 init_gfx_op(&op, vmode_g);
@@ -599,7 +597,6 @@ void
 vgafb_scroll(struct cursorpos win, struct cursorpos winsize
  , int lines, struct carattr ca)
 {
-vgafb_set_swcursor(0);
 if (!lines) {
 // Clear window
 vgafb_clear_char

[SeaBIOS] [PATCH] blockcmd: CMD_SCSI op is only used in 32bit mode

[Kevin O'Connor]

Reduce the size of the 16bit code slightly by recognizing that
CMD_SCSI is only used in 32bit mode.

Signed-off-by: Kevin O'Connor 
---
 src/hw/blockcmd.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c
index 093c5d7..f147100 100644
--- a/src/hw/blockcmd.c
+++ b/src/hw/blockcmd.c
@@ -118,6 +118,8 @@ scsi_fill_cmd(struct disk_op_s *op, void *cdbcmd, int 
maxcdb)
 cmd->count = cpu_to_be16(op->count);
 return GET_GLOBALFLAT(op->drive_gf->blksize);
 case CMD_SCSI:
+if (MODESEGMENT)
+return -1;
 memcpy(cdbcmd, op->cdbcmd, maxcdb);
 return op->blocksize;
 default:
@@ -129,13 +131,15 @@ scsi_fill_cmd(struct disk_op_s *op, void *cdbcmd, int 
maxcdb)
 int
 scsi_is_read(struct disk_op_s *op)
 {
-return op->command == CMD_READ || (op->command == CMD_SCSI && 
op->blocksize);
+return op->command == CMD_READ || (
+!MODESEGMENT && op->command == CMD_SCSI && op->blocksize);
 }
 
 // Check if a SCSI device is ready to receive commands
 int
 scsi_is_ready(struct disk_op_s *op)
 {
+ASSERT32FLAT();
 dprintf(6, "scsi_is_ready (drive=%p)\n", op->drive_gf);
 
 /* Retry TEST UNIT READY for 5 seconds unless MEDIUM NOT PRESENT is
@@ -181,6 +185,7 @@ scsi_is_ready(struct disk_op_s *op)
 int
 scsi_drive_setup(struct drive_s *drive, const char *s, int prio)
 {
+ASSERT32FLAT();
 struct disk_op_s dop;
 memset(&dop, 0, sizeof(dop));
 dop.drive_gf = drive;
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [BUG?] Doesn't retrieve control after coreboot payload return

[Kevin O'Connor]

On Tue, Aug 02, 2016 at 04:53:16PM +0200, Antonello Dettori wrote:
> On 02/08/16 16:39, Kevin O'Connor wrote:
> > On Tue, Aug 02, 2016 at 03:37:44PM +0200, Antonello Dettori wrote:
> > > Hi everyone.
> > > 
> > > I'm currently working on coreboot but I stumbled on a strange SeaBIOS
> > > behaviour.
> > > After executing a payload and returning control to the caller SeaBIOS
> > > crashes.
> > > 
> > > The problem is currently solved by rebooting before the payload returns 
> > > but
> > > doing so also makes chaining multiple payloads impossible so I'm trying to
> > > look into a solution.
> > The SeaBIOS payload support was not designed to robustly handle
> > returns from a payload.  It will crash if the payload alters the GDT,
> > overwrite the stack, or otherwise messes up the SeaBIOS state.  As far
> > as I know, payloads in general aren't designed to handle returns to
> > their caller.
> Some payloads can handle returns to the previous caller/payload.
> Would it be possible in theory to re-design the payload support so as to
> make it work (i.e. restoring the SeaBIOS state after returning)?
> Are there any reasons why it would be unfeasible?

Depends on what the payload is altering.  Which payload is it?

> > > I tried to increment the debug level to 8 to get more information and the
> > > attached log is what I got.
> > That log isn't from SeaBIOS.  It's unclear what hardware you have.  In
> > general, a serial port is the most robust way of obtaining a log.  See
> > also: http://www.seabios.org/Debugging
> > 
> > -Kevin
> I run the image on QEMU, didn't realise that SeaBIOS probably doesn't
> support QEMU debug port output.
> I'll try on a hardware target later.

SeaBIOS can support the qemu debug port, but it's probably easier to
compile seabios with serial support and instruct qemu to write the
serial log to a file: -serial file:foo.txt

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [QUESTION]stuck in SeaBIOS and vm can not be reset any more

[Kevin O'Connor]

On Tue, Aug 02, 2016 at 04:18:30AM +, Xulei (Stone) wrote:
> >On Fri, Jul 29, 2016 at 04:04:59AM +, Xulei (Stone) wrote:
> >> After one day, the vm is stuck. Looking from the following seabios log,
> >> it seems seabios stops at "PCI: Using 00:02.0 for primary VGA", and can
> >> not execute handle_smp() any more.
> >> What may be the reason?
> >
> >More debugging info would be necessary to find this problem.  You
> >could try reproducing and attaching gdb (
> >http://www.seabios.org/Debugging#Debugging_with_gdb_on_QEMU ).
> >Alternatively, a kvm trace log may help.
> >
> kvm trace (seems useful) indicates that cpu 0 keeps always to access 0x00b3 
> ioport.
> 0x00b3 is PORT_SMI_STATUS, so i guess my bios is stuck in the function
> smm_relocate_and_restore
> { 
>   ...
>   /* wait until SMM code executed */
> while (inb(PORT_SMI_STATUS) != 0x00)
>   ...
> }

I'd try adding dprintf() statements around all the code at the top of
smm_relocate_and_restore() and enable the dprintf() at the top of
handle_smi().

It would also be useful if you can extract the log from the last
two working reboots to compare it to the failed case.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [BUG?] Doesn't retrieve control after coreboot payload return

[Kevin O'Connor]

On Tue, Aug 02, 2016 at 03:37:44PM +0200, Antonello Dettori wrote:
> Hi everyone.
> 
> I'm currently working on coreboot but I stumbled on a strange SeaBIOS
> behaviour.
> After executing a payload and returning control to the caller SeaBIOS
> crashes.
> 
> The problem is currently solved by rebooting before the payload returns but
> doing so also makes chaining multiple payloads impossible so I'm trying to
> look into a solution.

The SeaBIOS payload support was not designed to robustly handle
returns from a payload.  It will crash if the payload alters the GDT,
overwrite the stack, or otherwise messes up the SeaBIOS state.  As far
as I know, payloads in general aren't designed to handle returns to
their caller.

> I tried to increment the debug level to 8 to get more information and the
> attached log is what I got.

That log isn't from SeaBIOS.  It's unclear what hardware you have.  In
general, a serial port is the most robust way of obtaining a log.  See
also: http://www.seabios.org/Debugging

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [QUESTION]stuck in SeaBIOS and vm can not be reset any more

[Kevin O'Connor]

On Fri, Jul 29, 2016 at 04:04:59AM +, Xulei (Stone) wrote:
> Hi, all:
> Recently when i try to reset a vm, I find it may be stuck in SeaBIOS.
> I use a shell script to continuously reset a vm to see what may happen.
> 
> #!/bin/bash
> while((1))
> do
> virsh reset VMNAME
> sleep 1
> done
> 
> After one day, the vm is stuck. Looking from the following seabios log,
> it seems seabios stops at "PCI: Using 00:02.0 for primary VGA", and can
> not execute handle_smp() any more.
> What may be the reason?

More debugging info would be necessary to find this problem.  You
could try reproducing and attaching gdb (
http://www.seabios.org/Debugging#Debugging_with_gdb_on_QEMU ).
Alternatively, a kvm trace log may help.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2 3/6] tpm: Extend tpm20_extend to support extending to multiple PCR banks

[Kevin O'Connor]

On Tue, Jul 26, 2016 at 11:19:50AM -0400, Stefan Berger wrote:
> Extend the tpm20_extend function to support extending a hash to
> multiple PCR banks. The sha1 hash that's being extended into the
> sha256 bank for example, will be filled with zero-bytes to the
> size of a sha256 hash.
[...]
> +/*
> + * Write the TPM2 tpml_digest_values data structure from the given hash.
> + * Follow the PCR bank configuration of the TPM and write the same hash
> + * in either truncated or zero-padded form in the areas of all the other
> + * hashes. For example, write the sha1 hash in the area of the sha256
> + * hash and fill the remaining bytes with zeros. Or truncate the sha256
> + * hash when writing it in the area of the sha1 hash.
> + *
> + * dest: destination buffer to write into; if NULL, nothing is written
> + * destlen: length of destination buffer
> + * pcrindex: the PCR index
> + * hash: the hash value to write
> + * hashAlg: the hash alogorithm determining the size of the hash
> + * count: pointer to a counter for how many entries were writte
> + *
> + * Returns the number of bytes needed in the buffer; -1 on fatal error
> + */
> +static int
> +tpm20_write_tpml_dig_values(u8 *dest, size_t destlen, u32 pcrindex,
> +const u8 *hash, u16 hashAlg)

So, if I understand this correctly, the current code creates a
"digest" with just a sha1 hash.  However, the hardware has a
description of what the digest should look like, and this patch takes
the current digest and produces the digest format desired by the
hardware.  Patch 6 does the same for the log.

If so, could the code instead build the digest according to the
hardware description instead of trying to reformat it after it is
built?  Specifically, the only callers of tpm_extend() and
tpm_log_event() produce a 'struct tcg_pcr_event2_sha1' with the digest
in the simple hash format - could these locations create
tcg_pcr_event2_sha1 in the desired hardware specified format initially
and thus avoid needing to reformat that digest?

[...]
> +if (dest && offset > destlen)
> +panic("buffer for tpml_digest_values is too small\n");

panic() should be avoided.  On real hardware if the BIOS were to
panic() it could effectively brick the machine.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2 1/6] tpm: Retrieve the PCR Bank configuration

[Kevin O'Connor]

On Tue, Jul 26, 2016 at 11:19:48AM -0400, Stefan Berger wrote:
> Implement tpm20_get_capability and retrieve the PCR Bank configuration
> from the TPM using this function.
[...]
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -24,6 +24,7 @@
>  #include "tcgbios.h"// tpm_*, prototypes
>  #include "util.h" // printf, get_keystroke
>  #include "stacks.h" // wait_threads, reset
> +#include "malloc.h" // malloc_high
>  
>  /
>   * TPM 1.2 commands
> @@ -76,6 +77,9 @@ static int TPM_has_physical_presence;
>  
>  static TPMVersion TPM_version;
>  
> +static u32 tpm20_pcr_selection_size;
> +static struct tpml_pcr_selection *tpm20_pcr_selection;
> +
>  static struct tcpa_descriptor_rev2 *
>  find_tcpa_by_rsdp(struct rsdp_descriptor *rsdp)
>  {
> @@ -181,6 +185,57 @@ tpm_log_event(struct tcg_pcr_event2_sha1 *entry, const 
> void *event
>  return 0;
>  }
>  
> +static int
> +tpm20_getcapability(u32 capability, u32 property, u32 count,
> +struct tpm_rsp_header *rsp, u32 rsize)

The tcgbios.c file is getting pretty large - 2200 lines after this
series.  This patch adds tpm20_getcapability() in the section of the
file currently labeled "ACPI TCPA table interface", but it seems very
similar to tpm12_getcapability() and to other wrappers around
tpmhw_transmit() that are located elsewhere in the C file.

I think it would help if the code could be segmented into like
functionality, or split into multiple C files of like functionality if
necessary.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] vgabios: Simplify scroll logic

[Kevin O'Connor]

On Thu, Jul 21, 2016 at 11:01:10AM -0400, Kevin O'Connor wrote:
> Introduce a new function vgafb_scroll() to scroll a window on the
> screen and update vgabios.c to use only that function for scrolling.
> This makes the low-level vgafb_move_chars() and vgafb_clear_chars()
> local to vgafb.c, and it simplifies the callers.

FYI, I have committed this change.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] How CPU executes bios code before copying bios to shadow ram?

[Kevin O'Connor]

On Tue, Jul 26, 2016 at 03:02:02PM +0800, Li Wang wrote:
> Hi all,
> I am reading seaBios code, and I have a question about the shadow memory
> copy part. In fw/shadow.c:make_bios_writable_intel() reads pam0 to see if
> shadow memory is already readable (if pam0's fourth bit is set), if pam0
> shows shadow memory is not readable running __make_bios_writable_intel from
> high-memory flash location (statements marked green below).
>   But in my understanding the entry point of bios is 0x:fff0, then it
> jumps to 0xf000:e05b, which points to memory space in shadowing, but before
> __make_bios_writable_intel copying bios from high-memory flash to shadow
> memory, shadow memory is disabled, so these codes are forwarded to
> high-memory flash, including code to read pam0 before invoking
> __make_bios_writable_intel (statement marked red below). Why these codes
> are not relocate to high-memory flash, but only the invocation of
> __make_bios_writable_intel is need to be relocated?
> If shadow ram is present and readable, how cpu execute bios codes in
> 0xf000: before copying them to shadow ram?

This code only runs on QEMU and is very specific to the quirky way
that QEMU implements the pam registers.  When emulation starts, QEMU
places a read-only copy of the code in 0xe-0x10.  When SeaBIOS
requests that 0xc-0x10 be read/writable ram by writing to the
pam registers in __make_bios_writable_intel(), then qemu converts the
region to uninitialized memory.  This is why
__make_bios_writable_intel() needs to run from the copy of the code in
the "flash" location at the end of the first 4Gig of ram.  The
make_bios_writable_intel() code can run in 0xe-0x10 because
prior to __make_bios_writable_intel() QEMU places a read-only copy of
the code there and after __make_bios_writable_intel() SeaBIOS has
restored the code by copying the code back to that ram.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH] vgabios: Simplify scroll logic

[Kevin O'Connor]

Introduce a new function vgafb_scroll() to scroll a window on the
screen and update vgabios.c to use only that function for scrolling.
This makes the low-level vgafb_move_chars() and vgafb_clear_chars()
local to vgafb.c, and it simplifies the callers.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgabios.c | 57 -
 vgasrc/vgabios.h |  6 ++---
 vgasrc/vgafb.c   | 70 ++--
 3 files changed, 64 insertions(+), 69 deletions(-)

diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index cfb6ba2..2c8cc79 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -198,15 +198,10 @@ write_teletype(struct cursorpos *pcp, struct carattr ca)
 if (pcp->y > nbrows) {
 pcp->y--;
 
-struct cursorpos dest = {0, 0, pcp->page};
-struct cursorpos src = {0, 1, pcp->page};
-struct cursorpos size = {GET_BDA(video_cols), nbrows};
-vgafb_move_chars(dest, src, size);
-
-struct cursorpos clr = {0, nbrows, pcp->page};
+struct cursorpos win = {0, 0, pcp->page};
+struct cursorpos winsize = {GET_BDA(video_cols), nbrows+1};
 struct carattr attr = {' ', 0, 0};
-struct cursorpos clrsize = {GET_BDA(video_cols), 1};
-vgafb_clear_chars(clr, attr, clrsize);
+vgafb_scroll(win, winsize, 1, attr);
 }
 }
 
@@ -400,6 +395,7 @@ handle_1005(struct bregs *regs)
 static void
 verify_scroll(struct bregs *regs, int dir)
 {
+// Verify parameters
 u8 ulx = regs->cl, uly = regs->ch, lrx = regs->dl, lry = regs->dh;
 u16 nbrows = GET_BDA(video_rows) + 1;
 if (lry >= nbrows)
@@ -410,41 +406,16 @@ verify_scroll(struct bregs *regs, int dir)
 int wincols = lrx - ulx + 1, winrows = lry - uly + 1;
 if (wincols <= 0 || winrows <= 0)
 return;
-
-u8 page = GET_BDA(video_page);
-int clearlines = regs->al, movelines = winrows - clearlines;
-if (!clearlines || movelines <= 0) {
-// Clear whole area.
-struct cursorpos clr = {ulx, uly, page};
-struct carattr attr = {' ', regs->bh, 1};
-struct cursorpos clrsize = {wincols, winrows};
-vgafb_clear_chars(clr, attr, clrsize);
-return;
-}
-
-if (dir > 0) {
-// Normal scroll
-struct cursorpos dest = {ulx, uly, page};
-struct cursorpos src = {ulx, uly + clearlines, page};
-struct cursorpos size = {wincols, movelines};
-vgafb_move_chars(dest, src, size);
-
-struct cursorpos clr = {ulx, uly + movelines, page};
-struct carattr attr = {' ', regs->bh, 1};
-struct cursorpos clrsize = {wincols, clearlines};
-vgafb_clear_chars(clr, attr, clrsize);
-} else {
-// Scroll down
-struct cursorpos dest = {ulx, uly + clearlines, page};
-struct cursorpos src = {ulx, uly, page};
-struct cursorpos size = {wincols, movelines};
-vgafb_move_chars(dest, src, size);
-
-struct cursorpos clr = {ulx, uly, page};
-struct carattr attr = {' ', regs->bh, 1};
-struct cursorpos clrsize = {wincols, clearlines};
-vgafb_clear_chars(clr, attr, clrsize);
-}
+int lines = regs->al;
+if (lines >= winrows)
+lines = 0;
+lines *= dir;
+
+// Scroll (or clear) window
+struct cursorpos win = {ulx, uly, GET_BDA(video_page)};
+struct cursorpos winsize = {wincols, winrows};
+struct carattr attr = {' ', regs->bh, 1};
+vgafb_scroll(win, winsize, lines, attr);
 }
 
 static void
diff --git a/vgasrc/vgabios.h b/vgasrc/vgabios.h
index 831f694..9fbfb14 100644
--- a/vgasrc/vgabios.h
+++ b/vgasrc/vgabios.h
@@ -126,10 +126,8 @@ extern struct video_func_static static_functionality;
 void init_gfx_op(struct gfx_op *op, struct vgamode_s *vmode_g);
 void handle_gfx_op(struct gfx_op *op);
 void *text_address(struct cursorpos cp);
-void vgafb_move_chars(struct cursorpos dest
-  , struct cursorpos src, struct cursorpos movesize);
-void vgafb_clear_chars(struct cursorpos dest
-   , struct carattr ca, struct cursorpos movesize);
+void vgafb_scroll(struct cursorpos win, struct cursorpos winsize
+  , int lines, struct carattr ca);
 void vgafb_write_char(struct cursorpos cp, struct carattr ca);
 struct carattr vgafb_read_char(struct cursorpos cp);
 void vgafb_write_pixel(u8 color, u16 x, u16 y);
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 58c60ad..bd1c8dd 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -346,7 +346,7 @@ handle_gfx_op(struct gfx_op *op)
 // Move characters when in graphics mode.
 static void
 gfx_move_chars(struct vgamode_s *vmode_g, struct cursorpos dest
-   , struct cursorpos src, struct cursorpos movesize)
+   , struct cursorpos movesize, int lines)
 {
 struct gfx_op op;
 init_gfx_op(&

Re: [SeaBIOS] [PATCH 3/3] tpm: Extend tpm20_extend to support extending to multiple PCR banks

[Kevin O'Connor]

On Tue, Jul 19, 2016 at 01:41:56PM -0400, Stefan Berger wrote:
> Extend the tpm20_extend function to support extending a hash to
> multiple PCR banks. The sha1 hash that's being extended into the
> sha256 bank for example, will be filled with zero-bytes to the
> size of a sha256 hash.
> 
> Signed-off-by: Stefan Berger 
> ---
>  src/std/tcg.h | 14 +++--
>  src/tcgbios.c | 99 
> ---
>  2 files changed, 105 insertions(+), 8 deletions(-)
> 
> diff --git a/src/std/tcg.h b/src/std/tcg.h
> index 1644684..9e4ef57 100644
> --- a/src/std/tcg.h
> +++ b/src/std/tcg.h
> @@ -100,6 +100,10 @@ enum irq_ids {
>  #define EV_IPL_PARTITION_DATA   14
>  
>  #define SHA1_BUFSIZE20
> +#define SHA256_BUFSIZE  32
> +#define SHA384_BUFSIZE  48
> +#define SHA512_BUFSIZE  64
> +#define SM3_256_BUFSIZE 32
>  
>  /* Input and Output blocks for the TCG BIOS commands */
>  
> @@ -381,6 +385,10 @@ struct tpm_res_sha1complete {
>  #define TPM2_RH_PLATFORM0x400c
>  
>  #define TPM2_ALG_SHA1   0x0004
> +#define TPM2_ALG_SHA256 0x000b
> +#define TPM2_ALG_SHA384 0x000c
> +#define TPM2_ALG_SHA512 0x000d
> +#define TPM2_ALG_SM3_2560x0012
>  
>  /* TPM 2 command tags */
>  #define TPM2_ST_NO_SESSIONS 0x8001
> @@ -442,8 +450,8 @@ struct tpm2_req_hierarchychangeauth {
>  } PACKED;
>  
>  struct tpm2_digest_value {
> -u16 hashalg; /* TPM2_ALG_SHA1 */
> -u8 sha1[SHA1_BUFSIZE];
> +u16 hashAlg; /* TPM2_ALG_SHA1 */
> +u8 hash[0]; /* size depends on hashAlg */
>  } PACKED;
>  
>  struct tpm2_req_extend {
> @@ -452,7 +460,7 @@ struct tpm2_req_extend {
>  u32 authblocksize;
>  struct tpm2_authblock authblock;
>  u32 count;
> -struct tpm2_digest_value digest;
> +struct tpm2_digest_value digest[0];
>  } PACKED;
>  
>  struct tpm2_req_clearcontrol {
> diff --git a/src/tcgbios.c b/src/tcgbios.c
> index 72ae3c6..acd0b71 100644
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -446,11 +446,94 @@ tpm12_extend(u32 pcrindex, const u8 *digest)
>  return 0;
>  }
>  
> +static int
> +tpm20_get_hash_buffersize(u16 hashAlg)
> +{
> +switch (hashAlg) {
> +case TPM2_ALG_SHA1:
> +return SHA1_BUFSIZE;
> +case TPM2_ALG_SHA256:
> +return SHA256_BUFSIZE;
> +case TPM2_ALG_SHA384:
> +return SHA384_BUFSIZE;
> +case TPM2_ALG_SHA512:
> +return SHA512_BUFSIZE;
> +case TPM2_ALG_SM3_256:
> +return SM3_256_BUFSIZE;
> +default:
> +return -1;
> +}
> +}
> +
> +/*
> + * Write the TPM2 PCR Extend 'body' containing the hashes. Follow
> + * the PCR bank configuration of the TPM.
> + *
> + * dest: destinatiobn buffer to write into; if NULL, nothing is written

Spelling typo.

> + * pcrindex: the PCR index
> + * hash: the hash value to write
> + * hashAlg: the hash alogorithm determining the size of the hash
> + * count: pointer to a counter for how many entries were writte
> + *
> + * Returns the number of bytes written into the buffer; -1 on fatal error
> + */
> +static int
> +tpm20_write_extend_body(char *dest, u32 pcrindex, const u8 *hash,
> +u16 hashAlg, u32 *count)
> +{
> +int offset = 0, hsize;
> +struct tpms_pcr_selection *sel = tpm20_pcr_selection->selections;
> +void *nsel, *end = (void *)tpm20_pcr_selection + 
> tpm20_pcr_selection_size;
> +int hash_size = tpm20_get_hash_buffersize(hashAlg);
> +
> +for (*count = 0;
> + *count < be32_to_cpu(tpm20_pcr_selection->count);
> + (*count)++) {
> +u8 sizeOfSelect = sel->sizeOfSelect;
> +
> +nsel = (void *)sel +
> +   offsetof(struct tpms_pcr_selection, pcrSelect) +
> +   sizeOfSelect;
> +if (nsel > end)
> +break;
> +
> +u8 idx = pcrindex >> 3;
> +u8 mask = (1 << (pcrindex & 7));
> +/* skip if PCR not used in this bank */
> +if (idx > sizeOfSelect || !(sel->pcrSelect[idx] & mask))
> +continue;
> +
> +hsize = tpm20_get_hash_buffersize(be16_to_cpu(sel->hashAlg));
> +if (hsize < 0) {
> +dprintf(DEBUG_tcg, "TPM is using an unsupported hash: %d\n",
> +be16_to_cpu(sel->hashAlg));
> +return -1;
> +}
> +
> +if (dest) {
> +struct tpm2_digest_value *v =
> +  (struct tpm2_digest_value *)&dest[offset];
> +v->hashAlg = sel->hashAlg;
> +memset(v->hash, 0, hsize);
> +memcpy(v->hash, hash, hash_size);
> +}
> +offset += sizeof(sel->hashAlg) + hsize;
> +sel = nsel;
> +}
> +
> +if (sel != end) {
> +dprintf(DEBUG_tcg, "Malformed pcr selection structure fron TPM\n");
> +return -1;
> +}
> +
> +return offset;
> +}
> +
>  static int tpm20_extend(u32 pcrindex, const u8 *digest, u16 hashAlg)
>  {
>  

Re: [SeaBIOS] [PATCH 1/3] tpm: Retrieve the PCR Bank configuration

[Kevin O'Connor]

On Tue, Jul 19, 2016 at 01:41:54PM -0400, Stefan Berger wrote:
> Implement tpm20_get_capability and retrieve the PCR Bank configuration
> from the TPM using this function.
> 
> Signed-off-by: Stefan Berger 
> ---
>  src/std/tcg.h | 29 +
>  src/tcgbios.c | 53 +
>  2 files changed, 82 insertions(+)
> 
> diff --git a/src/std/tcg.h b/src/std/tcg.h
> index c59f671..d60ee09 100644
> --- a/src/std/tcg.h
> +++ b/src/std/tcg.h
> @@ -394,12 +394,16 @@ struct tpm_res_sha1complete {
>  #define TPM2_CC_SelfTest0x143
>  #define TPM2_CC_Startup 0x144
>  #define TPM2_CC_StirRandom  0x146
> +#define TPM2_CC_GetCapability   0x17a
>  #define TPM2_CC_GetRandom   0x17b
>  #define TPM2_CC_PCR_Extend  0x182
>  
>  /* TPM 2 error codes */
>  #define TPM2_RC_INITIALIZE  0x100
>  
> +/* TPM 2 Capabilities */
> +#define TPM2_CAP_PCRS   0x0005
> +
>  /* TPM 2 data structures */
>  
>  struct tpm2b_stir {
> @@ -475,6 +479,31 @@ struct tpm2_req_hierarchycontrol {
>  u8 state;
>  } PACKED;
>  
> +struct tpm2_req_getcapability {
> +struct tpm_req_header hdr;
> +u32 capability;
> +u32 property;
> +u32 propertycount;
> +} PACKED;
> +
> +struct tpm2_res_getcapability {
> +struct tpm_rsp_header hdr;
> +u8 moreData;
> +u32 capability;
> +u8 data[0]; /* capability dependent data */
> +} PACKED;
> +
> +struct tpms_pcr_selection {
> +u16 hashAlg;
> +u8 sizeOfSelect;
> +u8 pcrSelect[0];
> +} PACKED;
> +
> +struct tpml_pcr_selection {
> +u32 count;
> +struct tpms_pcr_selection selections[0];
> +} PACKED;
> +
>  /* TPM 2 log entry */
>  
>  struct tpml_digest_values_sha1 {
> diff --git a/src/tcgbios.c b/src/tcgbios.c
> index 334d99b..a79b880 100644
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -24,6 +24,7 @@
>  #include "tcgbios.h"// tpm_*, prototypes
>  #include "util.h" // printf, get_keystroke
>  #include "stacks.h" // wait_threads, reset
> +#include "malloc.h" // malloc_high
>  
>  /
>   * TPM 1.2 commands
> @@ -76,6 +77,9 @@ static int TPM_has_physical_presence;
>  
>  static TPMVersion TPM_version;
>  
> +static u32 tpm20_pcr_selection_size;
> +static struct tpml_pcr_selection *tpm20_pcr_selection;
> +
>  static struct tcpa_descriptor_rev2 *
>  find_tcpa_by_rsdp(struct rsdp_descriptor *rsdp)
>  {
> @@ -669,6 +673,51 @@ err_exit:
>  }
>  
>  static int
> +tpm20_getcapability(u32 capability, u32 property, u32 count,
> +struct tpm_rsp_header *rsp, u32 rsize)
> +{
> +struct tpm2_req_getcapability trg = {
> +.hdr.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> +.hdr.totlen = cpu_to_be32(sizeof(trg)),
> +.hdr.ordinal = cpu_to_be32(TPM2_CC_GetCapability),
> +.capability = cpu_to_be32(capability),
> +.property = cpu_to_be32(property),
> +.propertycount = cpu_to_be32(count),
> +};
> +
> +u32 resp_size = rsize;
> +int ret = tpmhw_transmit(0, &trg.hdr, rsp, &resp_size,
> + TPM_DURATION_TYPE_SHORT);
> +ret = (ret ||
> +   rsize < be32_to_cpu(rsp->totlen)) ? -1 : 
> be32_to_cpu(rsp->errcode);
> +
> +dprintf(DEBUG_tcg, "TCGBIOS: Return value from sending 
> TPM2_CC_GetCapability = 0x%08x\n",
> +ret);
> +
> +return ret;
> +}
> +
> +static int
> +tpm20_get_pcrbanks(void)
> +{
> +u8 buffer[128];
> +struct tpm2_res_getcapability *trg =
> +  (struct tpm2_res_getcapability *)&buffer;
> +
> +int ret = tpm20_getcapability(TPM2_CAP_PCRS, 0, 8, &trg->hdr,
> +  sizeof(buffer));
> +if (ret)
> +return ret;
> +
> +tpm20_pcr_selection_size = be32_to_cpu(trg->hdr.totlen) -
> +   offsetof(struct tpm2_res_getcapability, data);
> +tpm20_pcr_selection = malloc_high(tpm20_pcr_selection_size);
> +memcpy(tpm20_pcr_selection, &trg->data, tpm20_pcr_selection_size);

It's necessary to check the result of malloc_high() for NULL.
(Otherwise, the memcpy could overwrite the memory at address zero,
which is the real-mode IDT.)

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 5/5] [wip] sercon: initial split-output implementation

[Kevin O'Connor]

On Fri, Jul 15, 2016 at 01:49:49PM +0200, Gerd Hoffmann wrote:
> > Finally, one high level observation is that we know there are a number
> > of quirks in various vgabios emulators.  For example, we know some
> > emulators don't handle certain 32bit instructions when in 16bit mode
> > (hence scripts/vgafixup.py), we know some versions of Windows use an
> > emulator that doesn't like some stack relative instructions (hence the
> > vgabios is compiled without -fomit-frame-pointer), and we know Windows
> > Vista doesn't like the extra stack in high ram (the skifree bug).  Any
> > thoughts on what happens with these quirks if the main seabios code
> > hooks int10?
> 
> Good question.  Do the emulators (both win, xorg) use the int10 vector
> set by seabios in the first place?  Or go they load the vgabios and run
> it, including the initialization, and use whatever entry point the init
> code sets up?  I suspect it is the latter.  But needs investigation and
> testing.

I think they just call the existing int10 handler.  In general, it's
not safe to rerun the vga init code.  Also, if they did run the init
it would lead to extra copies of the SeaVGABIOS version banners in the
debug logs, which I don't recall seeing.

> Also a serial console for windows guests isn't that useful, so I
> wouldn't worry too much about windows emulator issues.

It's not uncommon (at least on real hardware) to add sgabios to a
system for the boot menus, but then use a regular OS at runtime.  The
problem with the vga emulation quirks is that they often result in
mysterious system failures.

Have you considered implementing the serial support as a kind of
"serial seavgabios" instead of directly in seabios?  That would have
the advantage of pulling in all the existing vgabios quirk handling.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 1/5] std: add cp437 to unicode map

[Kevin O'Connor]

On Thu, Jul 14, 2016 at 10:52:58AM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  Makefile|   5 +-
>  src/std/cp437.c | 275 
> 
>  src/std/cp437.h |   1 +
>  3 files changed, 279 insertions(+), 2 deletions(-)
>  create mode 100644 src/std/cp437.c
>  create mode 100644 src/std/cp437.h
> 
> diff --git a/Makefile b/Makefile
> index 4930b3a..c11efe6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -34,7 +34,8 @@ SRCBOTH=misc.c stacks.c output.c string.c block.c cdrom.c 
> disk.c mouse.c kbd.c \
>  hw/usb.c hw/usb-uhci.c hw/usb-ohci.c hw/usb-ehci.c \
>  hw/usb-hid.c hw/usb-msc.c hw/usb-uas.c \
>  hw/blockcmd.c hw/floppy.c hw/ata.c hw/ramdisk.c \
> -hw/lsi-scsi.c hw/esp-scsi.c hw/megasas.c hw/mpt-scsi.c
> +hw/lsi-scsi.c hw/esp-scsi.c hw/megasas.c hw/mpt-scsi.c \
> +std/cp437.c

Lets put this file in src/ instead of src/std/.  (I think of std/ as a
location for published BIOS interface standards.)

Otherwise, looks good.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 5/5] [wip] sercon: initial split-output implementation

[Kevin O'Connor]

On Thu, Jul 14, 2016 at 10:53:02AM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  src/optionroms.c |  2 ++
>  src/romlayout.S  | 39 ++
>  src/sercon.c | 99 
> +++-
>  3 files changed, 111 insertions(+), 29 deletions(-)
> 
> diff --git a/src/optionroms.c b/src/optionroms.c
> index f9e9593..f08fcb1 100644
> --- a/src/optionroms.c
> +++ b/src/optionroms.c
> @@ -442,6 +442,8 @@ vgarom_setup(void)
>  }
>  
>  VgaROM = (void*)BUILD_ROM_START;
> +if (romfile_loadint("etc/sercon-enable", 0))
> +sercon_enable();

Minor nit, but why not unconditionally call sercon_enable() and let
sercon_enable() check romfile_loadint().

[...]
> --- a/src/romlayout.S
> +++ b/src/romlayout.S
> @@ -522,6 +522,45 @@ irqentry_arg:
>  DECL_IRQ_ENTRY hwpic1
>  DECL_IRQ_ENTRY hwpic2
>  
> +// hooked int10, for sercon
> +DECLFUNC entry_10_hooked
> +entry_10_hooked:
> + pushl $ handle_10
> +#if CONFIG_ENTRY_EXTRASTACK
> + // FIXME: too much cut+paste

I'm okay with the cut-and-paste.  But, another option would be to use
the iretw at the end of the existing irqentry_extrastack to implement
the ljmpw into the main vgabios.  Something like (totally untested):

entry_10_hooked:
pushfw  // Setup for iretw in irqentry_arg
pushl %cs:sercon_int10_hook_resume

pushl $handle_10
#if CONFIG_ENTRY_EXTRASTACK
jmp irqentry_arg_extrastack
#else
jmp irqentry_arg
#endif

Separately, have you considered choosing a separate entry point for
entry_10_hooked.  That is, changing the above pushl to
$handle_sercon_hooked and introducing that function in sercon.c.  It
seems it would reduce a number of "if (!sercon_splitmode())" checks in
the main code, as handle_sercon_hooked() could just use a smaller
switch statement and ignore requests it doesn't need to support.

Finally, one high level observation is that we know there are a number
of quirks in various vgabios emulators.  For example, we know some
emulators don't handle certain 32bit instructions when in 16bit mode
(hence scripts/vgafixup.py), we know some versions of Windows use an
emulator that doesn't like some stack relative instructions (hence the
vgabios is compiled without -fomit-frame-pointer), and we know Windows
Vista doesn't like the extra stack in high ram (the skifree bug).  Any
thoughts on what happens with these quirks if the main seabios code
hooks int10?

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] GeodeVGA is not initialized during SeaBIOS execution

[Kevin O'Connor]

On Mon, Jul 11, 2016 at 08:25:14PM +0300, Andrey Korolyov wrote:
> Awesome suggestion, turns out that the PCI_COMMAND_IO is not set.
> Removing the check entirely would lead to an incomplete PCI
> initialization but placing VGA ROM to vgaroms/seavgabios.rom could do
> the trick.

So, out of curiosity, does Geode SeaVGABIOS work on this board if one
of the above is done?  I haven't seen a recent confirmation that Geode
SeaVGABIOS still works.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] restore MSRs on S3 resume

[Kevin O'Connor]

On Thu, Jul 07, 2016 at 12:15:24PM -0400, Paolo Bonzini wrote:
> > On Thu, Jul 07, 2016 at 04:00:40PM +0200, Paolo Bonzini wrote:
> > > Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3
> > > resume.  Because these have to be applied to all processors, SMP setup
> > > has to be added to S3 resume.
> > > 
> > > There are two differences between the boot and resume paths.  First,
> > > romfile_* is not usable in the resume paths so we move the call out
> > > of smp_setup and into a new function smp_boot_setup.  Second,
> > > smp_msr has to be walked on the BSP as well, so we extract that
> > > out of handle_smp and into a new function smp_write_msrs.  Then,
> > > resume can call smp_write_msrs on the BSP followed by smp_setup to
> > > initialize the APs.
> > 
> > In general, looks good to me.  Are you okay with the slightly modified
> > patch below?  Three differences - check CONFIG_QEMU in smp_resume(),
> > only modify MaxCountCPUs during the init phase, and only export
> > smp_setup() and smp_resume() from smp.c.
> 
> Yes, of course.  Thanks!  Gerd, can you get this in QEMU 2.7 too?

Thanks.  I committed this change to master.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] GeodeVGA is not initialized during SeaBIOS execution

[Kevin O'Connor]

On Mon, Jul 11, 2016 at 02:23:12AM +0300, Andrey Korolyov wrote:
> Hi,
> 
> as per Kevin`s suggestion, I am posting initialization log (mixed one)
> from initialization of the GeodeLX board. As it could be clearly seen
> from the log, VGA option ROM was never called and executed during the
> initialization sequence, therefore issue with a lack of video output
> during SeaBIOS stage on this board looks somehow logical. [1] suggests
> that the problem existed long before, as most users of these boards
> are Alix owners who do not care about existence of the video output :)

Best guess is this check:

u16 cmd = pci_config_readw(pci->bdf, PCI_COMMAND);
if (!(cmd & PCI_COMMAND_IO && cmd & PCI_COMMAND_MEMORY))
return 0;

in optionroms.c:is_pci_vga() is being triggered.  You could see if
coreboot is not enabling these PCI flags, temporarily disable the
check in seabios, or use "vgaroms/seavgabios.rom" instead of
"pci1022,2081.rom" for the seavgabios rom.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] restore MSRs on S3 resume

[Kevin O'Connor]

On Thu, Jul 07, 2016 at 04:00:40PM +0200, Paolo Bonzini wrote:
> Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3
> resume.  Because these have to be applied to all processors, SMP setup
> has to be added to S3 resume.
> 
> There are two differences between the boot and resume paths.  First,
> romfile_* is not usable in the resume paths so we move the call out
> of smp_setup and into a new function smp_boot_setup.  Second,
> smp_msr has to be walked on the BSP as well, so we extract that
> out of handle_smp and into a new function smp_write_msrs.  Then,
> resume can call smp_write_msrs on the BSP followed by smp_setup to
> initialize the APs.

In general, looks good to me.  Are you okay with the slightly modified
patch below?  Three differences - check CONFIG_QEMU in smp_resume(),
only modify MaxCountCPUs during the init phase, and only export
smp_setup() and smp_resume() from smp.c.

BTW, this will conflict with Igor's pending series, but it doesn't
look too invasive.

-Kevin


commit 54e3a88609da074aaae2f04e592026ebf82169dc
Author: Paolo Bonzini 
Date:   Thu Jul 7 16:00:40 2016 +0200

smp: restore MSRs on S3 resume

Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3
resume.  Because these have to be applied to all processors, SMP setup
has to be added to S3 resume.

There are two differences between the boot and resume paths.  First,
romfile_* is not usable in the resume paths so we separate out the
remaining common code to a new smp_scan function.  Second, smp_msr has
to be walked on the BSP as well, so we extract that out of handle_smp
and into a new function smp_write_msrs.  Then, resume can call
smp_write_msrs on the BSP followed by smp_scan to initialize the APs.

Reported-by: Laszlo Ersek 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin O'Connor 

diff --git a/src/fw/smp.c b/src/fw/smp.c
index 6e706e4..719d51d 100644
--- a/src/fw/smp.c
+++ b/src/fw/smp.c
@@ -36,6 +36,15 @@ wrmsr_smp(u32 index, u64 val)
 smp_msr_count++;
 }
 
+static void
+smp_write_msrs(void)
+{
+// MTRR and MSR_IA32_FEATURE_CONTROL setup
+int i;
+for (i=0; i>24;
 dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
 
-// MTRR and MSR_IA32_FEATURE_CONTROL setup
-int i;
-for (i=0; ihttps://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 1/3] vgabios: Remove special case of dh==0xff in handle_1013()

[Kevin O'Connor]

On Mon, Jul 04, 2016 at 12:41:14PM -0400, Kevin O'Connor wrote:
> The original "lgpl vgabios" had a special case for dh==0xff in its
> int1013 (write string) code.  There does not appear to be any VGABIOS
> documentation supporting this as an externally available feature.  It
> appears this was for its own internal use when writing its strings to
> the screen.  SeaVGABIOS doesn't use this hack; this patch removes it
> from the code.

FYI, I committed this series.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 3/3] add serial console support

[Kevin O'Connor]

On Tue, Jul 05, 2016 at 05:07:08PM +0200, Gerd Hoffmann wrote:
> I also hacked up a patch to send output to both vga + serial:
> 
> https://www.kraxel.org/cgit/seabios/commit/?h=serial&id=3afd7b8bb96126b00989f3ae09f451bbec4f00f7
> 
> Not working stable though, seems to corrupt memory, not sure why.
> I'm storing the vgabios int10 vector at 0x5f, then chain-call into
> vgabios via "int 5f" instruction.  Anything obviously wrong with that?

Not sure why it isn't working.  Take a look at
vgaentry.S:entry_timer_hook and vgainit.c:hook_timer_irq() though.
That shows a working example of "hooking" an interrupt.

As an aside, I think it would be better if save/restoring the BDA
cursor position could be avoided.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] Issues building SeaBIOS with gcc > 4.8

[Kevin O'Connor]

On Tue, Jul 05, 2016 at 03:49:34PM +0200, Roger Pau Monné wrote:
> Hello,
> 
> I'm seeing the following issue when building SeaBIOS with gcc > 4.8:
> 
>   Compiling whole program out/ccode32flat.o
>   Compiling whole program out/code32seg.o
>   Compiling whole program out/ccode16.o
> /tmp//ccecsJuF.s: Assembler messages:
> /tmp//ccecsJuF.s:539: Error: register save offset not a multiple of 8
[...]
> /tmp//ccecsJuF.s:27692: Error: register save offset not a multiple of 8
> Makefile:154: recipe for target 'out/ccode16.o' failed
> gmake: *** [out/ccode16.o] Error 1
> 
> This works fine when using gcc 4.8, but fails in the same way with gcc 5.4. 
> The assembler version used is:
> 
> GNU assembler version 2.25.1 (x86_64-portbld-freebsd11.0) using BFD version 
> (GNU Binutils) 2.25.1
> 
> What causes this issue seems to be the usage of the "-m16" gcc flag, which 
> was introduced in gcc 4.9 AFAICT. The following workaround solves the issue, 
> but I would like to know if anyone else has seen this with newer gcc 
> versions, or if the gcc version from FreeBSD ports is broken.

I normally compile with gcc v5.3.1.  SeaBIOS is known to compile on
gcc v6 also.  I guess this is something strange in FreeBSD.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 3/3] add serial console support

[Kevin O'Connor]

On Mon, Jul 04, 2016 at 10:39:54PM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 

Not sure if this is still an RFC.  I think it needs to have a Kconfig
option.  It should probably also have runtime enable option.

[...]
> --- a/Makefile
> +++ b/Makefile
> @@ -29,7 +29,7 @@ LD32BIT_FLAG:=-melf_i386
>  
>  # Source files
>  SRCBOTH=misc.c stacks.c output.c string.c block.c cdrom.c disk.c mouse.c 
> kbd.c \
> -system.c serial.c clock.c resume.c pnpbios.c vgahooks.c pcibios.c apm.c \
> +system.c serial.c sercon.c clock.c resume.c pnpbios.c vgahooks.c 
> pcibios.c apm.c \

How about console.c instead of sercon.c?

[...]
> --- /dev/null
> +++ b/src/sercon.c
> @@ -0,0 +1,545 @@

There should be a copyright statement.

[...]
> +VARLOW u8 sercon_char[8];
> +VARLOW u8 sercon_attr[8];

Out of curiosity, is 8 needed for effective "uncluttering" or is it an
optimization?  (That is, would a size of 1 suffice for the common
case?)

[...]
> +static void sercon_lazy_putchar(u8 chr, u8 attr, u8 teletype)
> +{
> +u8 idx;
> +
> +if (cursor_pos_row() != GET_LOW(sercon_row_last) ||
> +cursor_pos_col() <  GET_LOW(sercon_col_last) ||
> +cursor_pos_col() >= GET_LOW(sercon_col_last) + 
> ARRAY_SIZE(sercon_char)) {
> +sercon_lazy_flush();
> +}
> +
> +idx = cursor_pos_col() - GET_LOW(sercon_col_last);
> +SET_LOW(sercon_char[idx], chr);
> +if (teletype)
> +cursor_pos_set(cursor_pos_row(),
> +   cursor_pos_col()+1);

This doesn't handle end of column wrapping properly.

> +else
> +SET_LOW(sercon_attr[idx], attr);
> +}

FYI, once sercon_1013 is implemented then 'teletype' is not mutually
exclusive with 'use_attr'.

[...]
> +static void sercon_1000(struct bregs *regs)
> +{
> +int mode, rows, cols;
> +
> +mode = regs->al;
> +switch (mode) {
> +case 0x03:
> +default:
> +cols = 80;
> +rows = 25;
> +regs->al = 0x30;
> +}
> +SET_LOW(sercon_col_last, 0);
> +SET_LOW(sercon_row_last, 0);
> +
> +cursor_pos_set(0, 0);
> +SET_BDA(video_mode, mode);
> +SET_BDA(video_cols, cols);
> +SET_BDA(video_rows, rows-1);
> +SET_BDA(cursor_type, 0x0007);
> +
> +sercon_term_reset();
> +sercon_term_clear_screen();

Should only clear screen if high-bit of regs->al not set.

> +/* Set text-mode cursor shape */
> +static void sercon_1001(struct bregs *regs)
> +{
> +/* dummy (add show/hide cursor?) */
> +}

Not sure if hiding the cursor is necessary, but it should
SET_BDA(cursor_type, regs->cx)

[...]
> +static void sercon_1003(struct bregs *regs)
> +{
> +regs->ax = 0;
> +regs->ch = 0;
> +regs->cl = 7;

regs->cx = GET_BDA(cursor_type)

Not sure why regs->ax is cleared.

[...]
> +static void sercon_1009(struct bregs *regs)
> +{
> +u16 count = regs->cx;
> +
> +if (count == 1) {
> +sercon_lazy_putchar(regs->al, regs->bl, 0);
> +
> +} else if (regs->al == 0x20 &&
> +   video_rows() * video_cols() == count &&
> +   cursor_pos_row() == 0 &&
> +   cursor_pos_col() == 0) {
> +/* override everything with spaces -> this is clear screen */
> +sercon_lazy_flush();
> +sercon_set_attr(regs->bl);
> +sercon_term_clear_screen();
> +
> +} else {
> +sercon_lazy_flush();
> +sercon_set_attr(regs->bl);
> +while (count) {
> +sercon_putchar(regs->al);

At a minimum that should be sercon_print_utf8(), though could
sercon_lazy_putchar() be used?

[...]
> +static void sercon_100e(struct bregs *regs)
> +{
> +switch (regs->al) {
> +case 7:
> +// beep
> +break;

sercon_putchar(0x07)

> +void VISIBLE16
> +sercon_10(struct bregs *regs)
> +{
> +if (!GET_LOW(sercon_port))
> +return;
> +
> +switch (regs->ah) {
> +case 0x00: sercon_1000(regs); break;
> +case 0x01: sercon_1001(regs); break;
> +case 0x02: sercon_1002(regs); break;
> +case 0x03: sercon_1003(regs); break;
> +case 0x06: sercon_1006(regs); break;
> +case 0x08: sercon_1008(regs); break;
> +case 0x09: sercon_1009(regs); break;
> +case 0x0e: sercon_100e(regs); break;
> +case 0x0f: sercon_100f(regs); break;
> +case 0x4f: sercon_104f(regs); break;
> +default:   sercon_10XX(regs); break;
> +}
> +}

I noticed sercon_1007, sercon_100a, and sercon_1013 aren't
implemented.  I guess they aren't used frequently, but they've been
around since the original IBM MDA adapter.

[...]
> +void VISIBLE16
> +sercon_check_event(void)

Not sure VISIBLE16 is needed.

> +{
> +u16 addr = GET_LOW(sercon_port);
> +u16 keycode;
> +u8 byte, count = 0;
> +int seq, chr, len;
> +
> +// check to see if there is a active serial port
> +if (!addr)
> +return;
> +if (inb(addr + SEROFF_LSR) == 0xFF)
> +return;
> +
> +// flush pending output
> +sercon_lazy_flush();
> +
> +// read all available data
> +whi

Re: [SeaBIOS] [PATCH 1/3] std: add cp437 to unicode map

[Kevin O'Connor]

On Mon, Jul 04, 2016 at 10:39:52PM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 

Nice!

[...]
> --- /dev/null
> +++ b/src/std/cp437.h

Instead of making a header file and including it in an array in the C
code I think it would be better to instead introduce cp437.c with:

u16 sercon_cp437[256] VARFSEG = {
   ...
};

[...]
> +[ 0x7f ] = 0x007f, //  DELETE

That should be 0x2302 (see the wikipedia page).

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC PATCH 1/2] serial console, output

[Kevin O'Connor]

On Mon, Jul 04, 2016 at 06:03:30PM +0200, Paolo Bonzini wrote:
> On 04/07/2016 18:00, Kevin O'Connor wrote:
> > Does anyone know where one can find the original svn commit history
> > for sgabios?  Seems the original google code repo is no longer
> > present.
> 
> There was no history as far as I remember, mostly a code drop.

Ah, right.

I found what I was looking for though - it was in the sgabios
design.txt file instead of the revision history:


Summary of new enhancements
---
SGABIOS now keeps a log of the last 256 characters written to
the screen and where they were written in the event an application
like lilo asks for the current character under the cursor.  These
are currently stored in a 1KB EBDA allocation which can be expanded
as needed.  This method avoids having to store a 64KB buffer for
the largest possible serial terminal supported (255x255).


So, if I read the above correctly, it was lilo that inspired the
"feature".  Anyway, something to keep in mind.

BTW, lots of good info in the rest of design.txt.

Cheers,
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 3/3] vgabios: Simplify set_cursor_pos()

[Kevin O'Connor]

Rework set_cursor_pos() to be slightly simpler.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgabios.c | 28 ++--
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index 2593e03..cfb6ba2 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -83,27 +83,19 @@ set_cursor_shape(u16 cursor_type)
 static void
 set_cursor_pos(struct cursorpos cp)
 {
-u8 page = cp.page, x = cp.x, y = cp.y;
-
-// Should not happen...
-if (page > 7)
+if (cp.page > 7)
+// Should not happen...
 return;
 
-vgafb_set_swcursor(0);
-
-// Bios cursor pos
-SET_BDA(cursor_pos[page], (y << 8) | x);
-
-if (!CONFIG_VGA_STDVGA_PORTS)
-return;
-
-// Set the hardware cursor
-u8 current = GET_BDA(video_page);
-if (cp.page != current)
-return;
+if (cp.page == GET_BDA(video_page)) {
+// Update cursor in hardware
+vgafb_set_swcursor(0);
+if (CONFIG_VGA_STDVGA_PORTS)
+stdvga_set_cursor_pos((int)text_address(cp));
+}
 
-// Calculate the memory address
-stdvga_set_cursor_pos((int)text_address(cp));
+// Update BIOS cursor pos
+SET_BDA(cursor_pos[cp.page], (cp.y << 8) | cp.x);
 }
 
 struct cursorpos
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 2/3] vgabios: Don't check for special case of page==0xff on external calls

[Kevin O'Connor]

The original "lgpl vgabios" internally used page=0xff as a mechanism
for specifying the current page.  It also would allow int1013 calls to
externally specify bh==0xff for the current page.  However, there is
no documentation supporting this as an externally available feature.
SeaVGABIOS does not need the internal shortcut; this patch removes the
code.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgabios.c | 14 --
 vgasrc/vgafb.c   |  2 +-
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index 8b50192..2593e03 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -109,16 +109,10 @@ set_cursor_pos(struct cursorpos cp)
 struct cursorpos
 get_cursor_pos(u8 page)
 {
-if (page == 0xff)
-// special case - use current page
-page = GET_BDA(video_page);
-if (page > 7) {
-struct cursorpos cp = { 0, 0, 0xfe };
-return cp;
-}
+if (page > 7)
+return (struct cursorpos) { 0, 0, 0 };
 u16 xy = GET_BDA(cursor_pos[page]);
-struct cursorpos cp = {xy, xy>>8, page};
-return cp;
+return (struct cursorpos) { xy, xy>>8, page };
 }
 
 static void
@@ -555,7 +549,7 @@ handle_100e(struct bregs *regs)
 // Ralf Brown Interrupt list is WRONG on bh(page)
 // We do output only on the current page !
 struct carattr ca = {regs->al, regs->bl, 0};
-struct cursorpos cp = get_cursor_pos(0xff);
+struct cursorpos cp = get_cursor_pos(GET_BDA(video_page));
 write_teletype(&cp, ca);
 set_cursor_pos(cp);
 }
diff --git a/vgasrc/vgafb.c b/vgasrc/vgafb.c
index 5d1ecc9..58c60ad 100644
--- a/vgasrc/vgafb.c
+++ b/vgasrc/vgafb.c
@@ -674,7 +674,7 @@ vgafb_set_swcursor(int enable)
 struct vgamode_s *vmode_g = get_current_mode();
 if (!vmode_g)
 return;
-struct cursorpos cp = get_cursor_pos(0xff);
+struct cursorpos cp = get_cursor_pos(GET_BDA(video_page));
 if (cp.x >= GET_BDA(video_cols) || cp.y > GET_BDA(video_rows)
 || GET_BDA(cursor_type) >= 0x2000)
 // Cursor not visible
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 1/3] vgabios: Remove special case of dh==0xff in handle_1013()

[Kevin O'Connor]

The original "lgpl vgabios" had a special case for dh==0xff in its
int1013 (write string) code.  There does not appear to be any VGABIOS
documentation supporting this as an externally available feature.  It
appears this was for its own internal use when writing its strings to
the screen.  SeaVGABIOS doesn't use this hack; this patch removes it
from the code.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vgabios.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index f07e85b..8b50192 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -1030,13 +1030,7 @@ handle_1012(struct bregs *regs)
 static void noinline
 handle_1013(struct bregs *regs)
 {
-struct cursorpos cp;
-if (regs->dh == 0xff)
-// if row=0xff special case : use current cursor position
-cp = get_cursor_pos(regs->bh);
-else
-cp = (struct cursorpos) {regs->dl, regs->dh, regs->bh};
-
+struct cursorpos cp = {regs->dl, regs->dh, regs->bh};
 u16 count = regs->cx;
 u8 *offset_far = (void*)(regs->bp + 0);
 u8 attr = regs->bl;
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC PATCH 1/2] serial console, output

[Kevin O'Connor]

On Mon, Jul 04, 2016 at 11:26:48AM -0400, Kevin O'Connor wrote:
> At one point I looked through the sgabios code and was able to
> understand how it did caching.  I'll take another look and see if I
> can describe it.

So, if I read the sgabios code correctly, it allocates a buffer of:

struct { u8 x, y; char c; } logbuf[256];
int logbuf_offset;

Every character sent on the serial port is appended to "logbuf" in
order, wrapping if necessary: logbuf[logbuf_offset++ % 256] = x, y, c.
On a read, it scans backwards from logbuf_offset to find the last
update to that cell.

Interestingly, it doesn't store the attribute with the character -
it's int1008 handler just returns the last attribute used anywhere on
the screen.

The code is only used if it is the sole vga code (as opposed to being
used in addition to an existing vgabios).

Does anyone know where one can find the original svn commit history
for sgabios?  Seems the original google code repo is no longer
present.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC PATCH 2/2] serial console, input

[Kevin O'Connor]

On Mon, Jul 04, 2016 at 11:16:45AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > +#define FLAG_CTRL  (1<<0)
> > > +#define FLAG_SHIFT (1<<1)
> > > +
> > > +VARLOW struct {
> > > +u8 flags;
> > > +u8 scancode;
> > > +} termchr[256] = {
> > > +[ '1'] = { .scancode = 0x02,  },
> > 
> > I think this table should be generated at runtime from
> > kbd.c:scan_to_keycode[].  Since it doesn't change at runtime,
> > malloc_fseg() / GET_GLOBAL() could be used instead of VARLOW.
> 
> Ah, ok.  Didn't notice this one can be used for ascii -> scancode
> lookups.
> 
> I'm wondering whenever it makes sense to build a reverse table.  We
> could simply search scan_to_keycode[] instead.  Sure it is slower than a
> table lookup, but still _way_ faster than any human can type, and we
> would save some real-mode memory.

Agreed.

[...]
> > There's a lot of complexity to implement buffering for
> > that unusual case.  I wonder if the buffer could be avoided - I played
> > with it a little and came up with the below (totally untested).  I'm
> > not sure if it's an improvement.
> 
> Hmm, so you avoid the buffer by maintaining an index into termseq and
> matched byte count instead.  Hmm.  Yes, avoids the buffer and also a few
> cpu cycles because you can continue searching where you left off.  I
> find the code flow harder to follow though.

Right.  Yeah, also not sure it's an improvement.

Does the original code flush the multi-byte sequence on a timeout?  I
suspect it is important that one can hit ESC without having to type
another key.  Also, I'd prefer to avoid backwards gotos if possible.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC PATCH 1/2] serial console, output

[Kevin O'Connor]

On Mon, Jul 04, 2016 at 02:46:24PM +0200, Gerd Hoffmann wrote:
> On Mo, 2016-07-04 at 11:11 +0200, Paolo Bonzini wrote:
> > On 04/07/2016 10:16, Gerd Hoffmann wrote:
> > > +static void sercon_set_color(u8 fg, u8 bg, u8 bold)
> > > +{
> > > +sercon_putchar('\x1b');
> > > +sercon_putchar('[');
> > > +sercon_putchar('0');
> > Add a sercon_putstr perhaps?
> 
> We run in real mode, so passing around pointers isn't that easy.
> Given this would make sense for constant strings only (like the 4-byte
> clear-screen sequence) and not so much for variable stuff we might put
> all strings in the global segment.
> 
> Would this ...
> 
> void sercon_putchar(char *ptr)
> {
> char c = GET_GLOBAL(ptr[0]);
> [ ... ]
> 
> ... work?

Yes.  See output.c:puts_cs() as an example.  It only works if it's a
constant string (as opposed to a string built on the stack).

> > > Maybe we can reuse the output buffer which we have anyway.  Logic needs
> > > reworked a bit.  We can't just clear characters after printing them out
> > > if we want be able to read them later, so we need a separate
> > > pending-updates bit.  Also should be bigger I guess, maybe 80 chars so
> > > it can cover a complete line.
> > 
> > 80x25 is just 2K...  Perhaps it's simpler to just allocate the whole
> > video buffer from the UMB or EBDA when serial console is in use?
> 
> 4k, we need both character and attribute.  But, yes, if we can allocate
> that somewhere in real mode address space without running into memory
> pressure this might be the best option.

Unfortunately, the screen can be larger than 80x25.

If a large buffer is desired, it's also possible to malloc_high() it
and then use either: call32() to access it, or int1587 to read/write
it (see ramdisk.c:ramdisk_copy() as an example).  Either way it seems
ugly.

At one point I looked through the sgabios code and was able to
understand how it did caching.  I'll take another look and see if I
can describe it.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC PATCH 2/2] serial console, input

[Kevin O'Connor]

On Fri, Jul 01, 2016 at 01:07:39PM -0400, Kevin O'Connor wrote:
> If I understand correctly, most keys are sent on the serial port as
> single bytes, but there are a few keys that are sent as multi-byte
> sequences.  There's a lot of complexity to implement buffering for
> that unusual case.  I wonder if the buffer could be avoided - I played
> with it a little and came up with the below (totally untested).  I'm
> not sure if it's an improvement.

The version below might be slightly easier to understand (still
totally untested).

-Kevin


u8 multibyte_read_count VARLOW;
u8 multibyte_read_pos VARLOW;

void
sercon_check_event(void)
{
u16 addr = GET_LOW(sercon_port);
...

// read and process data
int readdata = 0;
while (inb(addr + SEROFF_LSR) & 0x01) {
u8 byte = inb(addr + SEROFF_DATA);
readdata = 1;
int ret = sercon_check_multibyte(byte);
if (ret)
// byte part of multi-byte sequence
continue;
if (byte == 0x1b) {
// Start multi-byte sequence check
SET_LOW(multibyte_read_count, 1);
continue;
}
// Send normal key
sercon_sendkey(GET_LOW(termchr[chr].scancode), 
GET_LOW(termchr[chr].flags));
}

if (!readdata && GET_LOW(multibyte_read_count))
// Too long to read multi-byte sequence - must flush
dump_multibyte_sequence();
}

static int
sercon_check_multibyte(u8 byte)
{
int mb_count = GET_LOW(multibyte_read_count);
if (!mb_count)
// Not in a multi-byte sequence
return 0;
int mb_pos = GET_LOW(multibyte_read_pos);
while (GET_GLOBAL(termseq[mb_pos].seq[mb_count-1]) != byte) {
// Byte didn't match this sequence - find a sequence that does
mb_pos++;
if (mb_pos >= ARRAY_SIZE(termseq)
|| memcmp_far(GLOBAL_SEG, termseq[mb_pos-1].seq
  , GLOBAL_SEG, termseq[mb_pos].seq, mb_count-1) != 0)
// No match - must flush previusly queued keys
dump_multibyte_sequence();
return 0;
}
}
mb_count++;
if (!GET_GLOBAL(termseq[mb_pos].seq[mb_count-1])) {
// sequence complete - send key
sercon_sendkey(GET_GLOBAL(termseq[seq].scancode), 0);
mb_count = mb_pos = 0;
}
SET_LOW(multibyte_read_count, mb_count);
SET_LOW(multibyte_read_pos, mb_pos);
return 1;
}

static void
dump_multibyte_sequence(void)
{
sercon_sendkey(GET_LOW(termchr[0x1b].scancode), 
GET_LOW(termchr[0x1b].flags));
int i, mb_count = GET_LOW(multibyte_read_count);
for (i=0; ihttps://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC PATCH 2/2] serial console, input

[Kevin O'Connor]

On Fri, Jul 01, 2016 at 12:54:31PM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  src/clock.c  |   1 +
>  src/serial.c | 255 
> +++
>  src/util.h   |   1 +
>  3 files changed, 257 insertions(+)
> 
> diff --git a/src/clock.c b/src/clock.c
> index e83e0f3..e44e112 100644
> --- a/src/clock.c
> +++ b/src/clock.c
> @@ -295,6 +295,7 @@ clock_update(void)
>  floppy_tick();
>  usb_check_event();
>  ps2_check_event();
> +sercon_check_event();
>  }
>  
>  // INT 08h System Timer ISR Entry Point
> diff --git a/src/serial.c b/src/serial.c
> index 74b91bb..d72dd01 100644
> --- a/src/serial.c
> +++ b/src/serial.c
> @@ -655,3 +655,258 @@ void sercon_enable(void)
>  outb(0x01, addr + 0x02);   // enable fifo
>  enable_vga_console();
>  }
> +
> +/
> + * serial input
> + /
> +
> +VARLOW u8 rx_buf[16];
> +VARLOW u8 rx_bytes;
> +
> +VARLOW struct {
> +char seq[4];
> +u8 len;
> +u8 scancode;
> +} termseq[] = {
> +{ .seq = "OP", .len = 2, .scancode = 0x3b },// F1
> +{ .seq = "OQ", .len = 2, .scancode = 0x3c },// F2
> +{ .seq = "OR", .len = 2, .scancode = 0x3d },// F3
> +{ .seq = "OS", .len = 2, .scancode = 0x3e },// F4
> +{ .seq = "[A", .len = 2, .scancode = 0xc8 },// up
> +{ .seq = "[B", .len = 2, .scancode = 0xd0 },// down
> +{ .seq = "[C", .len = 2, .scancode = 0xcd },// right
> +{ .seq = "[D", .len = 2, .scancode = 0xcb },// left
> +};

It would be preferable to mark constant data with "static VAR16"
instead of VARLOW.

> +
> +#define FLAG_CTRL  (1<<0)
> +#define FLAG_SHIFT (1<<1)
> +
> +VARLOW struct {
> +u8 flags;
> +u8 scancode;
> +} termchr[256] = {
> +[ '1'] = { .scancode = 0x02,  },

I think this table should be generated at runtime from
kbd.c:scan_to_keycode[].  Since it doesn't change at runtime,
malloc_fseg() / GET_GLOBAL() could be used instead of VARLOW.

[...]
> +static void sercon_sendkey(u8 scancode, u8 flags)
> +{
> +if (flags & FLAG_CTRL)
> +process_key(0x1d);
> +if (flags & FLAG_SHIFT)
> +process_key(0x2a);
> +
> +if (scancode & 0x80) {
> +process_key(0xe0);
> +process_key(scancode & ~0x80);
> +process_key(0xe0);
> +process_key(scancode);
> +} else {
> +process_key(scancode);
> +process_key(scancode | 0x80);
> +}
> +
> +if (flags & FLAG_SHIFT)
> +process_key(0x2a | 0x80);
> +if (flags & FLAG_CTRL)
> +process_key(0x1d | 0x80);
> +}

Is it necessary to use process_key() here instead of injecting the
keycode directly with enqueue_key()?  I think the only difference is
the CONFIG_KBD_CALL_INT15_4F stuff and I'm not sure if anything
interesting needs that.

> +
> +void VISIBLE16
> +sercon_check_event(void)

Does this need VISIBLE16?

> +{
> +u16 addr = GET_LOW(sercon_port);
> +u8 byte, scancode, flags, count = 0;
> +int seq, chr, len;
> +
> +// check to see if there is a active serial port
> +if (!addr)
> +return;
> +if (inb(addr + SEROFF_LSR) == 0xFF)
> +return;
> +
> +// flush pending output
> +sercon_flush_lazy();
> +
> +// read all available data
> +while (inb(addr + SEROFF_LSR) & 0x01) {
> +byte = inb(addr + SEROFF_DATA);
> +if (GET_LOW(rx_bytes) < sizeof(rx_buf)) {
> +SET_LOW(rx_buf[rx_bytes], byte);
> +SET_LOW(rx_bytes, GET_LOW(rx_bytes) + 1);
> +count++;
> +}
> +}
> +
> +next_char:
> +// no (more) input data
> +if (!GET_LOW(rx_bytes))
> +return;
> +
> +// lookup escape sequences
> +if (GET_LOW(rx_bytes) > 1 && GET_LOW(rx_buf[0]) == 0x1b) {
> +for (seq = 0; seq < ARRAY_SIZE(termseq); seq++) {
> +len = GET_LOW(termseq[seq].len);
> +if (GET_LOW(rx_bytes) < len + 1)
> +continue;
> +for (chr = 0; chr < len; chr++) {
> +if (GET_LOW(termseq[seq].seq[chr]) != GET_LOW(rx_buf[chr + 
> 1]))
> +break;
> +}
> +if (chr == len) {
> +scancode = GET_LOW(termseq[seq].scancode);
> +sercon_sendkey(scancode, 0);
> +shiftbuf(len + 1);
> +goto next_char;
> +}
> +}
> +}
> +
> +// Seems we got a escape sequence we didn't recognise.
> +//  -> If we received data wait for more, maybe it is just incomplete.
> +if (GET_LOW(rx_buf[0]) == 0x1b && count)
> +return;
> +
> +// Handle input as individual chars.
> +chr = GET_LOW(rx_buf[0]);
> +scancode = GET_LOW(termchr[chr].scancode);
> +flags = GET_LOW(termchr[chr].flags);
> +if (scancode)
> +sercon_sendkey(scancode, flags);
> +shiftbuf(1);
> 

Re: [SeaBIOS] [RFC PATCH 1/2] serial console, output

[Kevin O'Connor]

On Fri, Jul 01, 2016 at 12:54:30PM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 

Thanks.  See my comments below.

[...]
> --- a/src/misc.c
> +++ b/src/misc.c
> @@ -11,6 +11,7 @@
>  #include "output.h" // debug_enter
>  #include "stacks.h" // call16_int
>  #include "string.h" // memset
> +#include "util.h" // serial_10
>  
>  #define PORT_MATH_CLEAR0x00f0
>  
> @@ -57,6 +58,7 @@ handle_10(struct bregs *regs)
>  {
>  debug_enter(regs, DEBUG_HDL_10);
>  // don't do anything, since the VGA BIOS handles int10h requests
> +sercon_10(regs);
>  }

Might as well remove handle_10 and call sercon_10 directly from
romlayout.S.

[...]
> --- a/src/serial.c
> +++ b/src/serial.c
> @@ -315,3 +315,343 @@ handle_17(struct bregs *regs)
>  default:   handle_17XX(regs); break;
>  }
>  }
> +
> +/
> + * serial console output
> + /

I think this code should go in a new c file and not modify serial.c.

[...]
> +VARLOW u8 sercon_mode;
> +VARLOW u8 sercon_cols;
> +VARLOW u8 sercon_rows;
> +
> +VARLOW u8 sercon_row;
> +VARLOW u8 sercon_col;

I think the code should use the BDA equivalents of the above instead
of declaring them in the private VARLOW space.  Some old programs may
rely on the equivalents in the BDA being updated.

> +
> +/*
> + * We have a small output buffer here, for lazy output.  That allows
> + * to avoid a whole bunch of control sequences for pointless cursor
> + * moves, so when logging the output it'll be *alot* less cluttered.
> + *
> + * sercon_char/attr  is the actual output buffer.
> + * sercon_col_lazy   is the column of the terminal's cursor, typically
> + *   a few positions left of sercon_col.
> + * sercon_attr_last  is the most recent attribute sent to the terminal.
> + */
> +VARLOW u8 sercon_attr_last;
> +VARLOW u8 sercon_col_lazy;
> +VARLOW u8 sercon_char[8];
> +VARLOW u8 sercon_attr[8];
> +
> +VARLOW u8 sercon_cmap[8] = { '0', '4', '2', '6', '1', '5', '3', '7' };

For constant data (sercon_cmap) it would be preferable to use "static
VAR16" (see kbd.c:scan_to_keycode[] and mouse.c:sample_rates[] as
examples) and use GET_GLOBAL() instead of GET_LOW().

[...]
> +static void sercon_print_char(u8 chr, u8 attr)
> +{
> +if (chr != 0x00) {
> +sercon_set_attr(attr);
> +sercon_putchar(chr);
> +} else {
> +/* move cursor right */
> +sercon_putchar('\x1b');
> +sercon_putchar('[');
> +sercon_putchar('C');
> +}
> +}
> +
> +static void sercon_flush_lazy(void)
> +{
> +u8 count = GET_LOW(sercon_col) - GET_LOW(sercon_col_lazy);
> +u8 pos = 0;
> +
> +if (!count && !GET_LOW(sercon_attr[0]))
> +return;
> +
> +while (count) {
> +sercon_print_char(GET_LOW(sercon_char[pos]),
> +  GET_LOW(sercon_attr[pos]));
> +count--;
> +pos++;
> +}
> +
> +if (pos < ARRAY_SIZE(sercon_char) && GET_LOW(sercon_char[pos])) {
> +sercon_print_char(GET_LOW(sercon_char[pos]),
> +  GET_LOW(sercon_attr[pos]));
> +/* move cursor left */
> +sercon_putchar('\x1b');
> +sercon_putchar('[');
> +sercon_putchar('D');
> +}
> +
> +SET_LOW(sercon_col_lazy, GET_LOW(sercon_col));
> +for (pos = 0; pos < ARRAY_SIZE(sercon_char); pos++) {
> +SET_LOW(sercon_attr[pos], 0x07);
> +SET_LOW(sercon_char[pos], 0x00);
> +}
> +}

So, if I understand the above correctly, it's a buffer of recent
updates used to reduce serial bandwidth (and unclutter serial logs) in
the case where the application code issues a bunch of cursor moves /
cell updates that are mostly redundant.

> +/* Set video mode */
> +static void sercon_1000(struct bregs *regs)
> +{
> +SET_LOW(sercon_mode, regs->al);
> +switch (regs->al) {
> +case 0x03:
> +default:
> +SET_LOW(sercon_cols, 80);
> +SET_LOW(sercon_rows, 25);
> +regs->al = 0x30;
> +}
> +SET_LOW(sercon_row, 0);
> +SET_LOW(sercon_col, 0);
> +SET_LOW(sercon_col_lazy, 0);
> +sercon_init();
> +}

FYI, the screen should only be cleared if the high bit of the mode is
not set.  I do wonder if more of the vgabios interface code from
vgasrc/vgabios.c could be reused here.

> +/* Set cursor position */
> +static void sercon_1002(struct bregs *regs)
> +{
> +u8 row = regs->dh;
> +u8 col = regs->dl;
> +
> +if (row == GET_LOW(sercon_row) &&
> +col >= GET_LOW(sercon_col_lazy) &&
> +col <  GET_LOW(sercon_col_lazy) + ARRAY_SIZE(sercon_char)) {
> +SET_LOW(sercon_col, col);
> +if (col+1 == GET_LOW(sercon_col_lazy) + ARRAY_SIZE(sercon_char))
> +sercon_flush_lazy();
> +} else {
> +sercon_flush_lazy();
> +if (row == GET_LOW(sercon_row) && col == 0) {
> +sercon_putchar('\r');
> +} else {
> +serc

Re: [SeaBIOS] [PATCH v2 0/4] support booting more than 255 CPUs with QEMU

[Kevin O'Connor]

On Fri, Jun 24, 2016 at 10:12:12AM +0200, Paolo Bonzini wrote:
> On 24/06/2016 10:06, Gerd Hoffmann wrote:
> >>> If there are any other suggestions for cherry-picks please speak up now.
> >>
> >> I would like to have "[PATCH v3] fw/msr_feature_control: add support to
> >> set MSR_IA32_FEATURE_CONTROL", please.
> > 
> > Hmm, the qemu side for that one isn't merged yet, right?
> > Is it queued to land in 2.7 already?
> 
> The KVM side has been merged, and the LMCE patches are on track for
> being merged in 2.7 during soft freeze.  Even if LMCE patches were not
> merged, I would extract the patch that creates the fw_cfg file and use
> it for nested virtualization only.

The v3 patch looks fine to me.  Normally I prefer to commit to SeaBIOS
after the corresponding QEMU commit, but if it is easier to commit
this one to SeaBIOS first I'm also fine with that.  Gerd - I'll leave
this one for you to commit to master as you see fit.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 0/2] virtio: pci cfg access

[Kevin O'Connor]

On Fri, Jun 17, 2016 at 01:09:05PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> I'm sure I had sent this before, but appearently totally forgot about
> it.  Just tested booting with virtio bars mapped above 4G, didn't work.
> Investigated, found this bitroting in a local branch.  Undusted it,
> rebased to latest master with some fixups, and here it is ...

Looks good to me.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] fw/msr_feature_control: add support to set MSR_IA32_FEATURE_CONTROL

[Kevin O'Connor]

On Fri, Jun 17, 2016 at 03:20:10PM +0800, Haozhong Zhang wrote:
> OS usually expects BIOS to set certain bits in MSR_IA32_FEATURE_CONTROL
> for some features (e.g. VMX and LMCE). QEMU provides a fw_cfg file
> "etc/msr_feature_control" to advise bits that should be set in
> MSR_IA32_FEATURE_CONTROL. If this file exists, SeaBIOS will set the
> advised bits in that MSR.

Thanks - see my comments below.

> --- /dev/null
> +++ b/src/fw/msr_feature_control.c
> @@ -0,0 +1,16 @@
> +#include "util.h" // msr_feature_control_setup, wrmsr_smp
> +#include "romfile.h" // romfile_find
> +
> +#define MSR_IA32_FEATURE_CONTROL 0x003a
> +
> +void msr_feature_control_setup(void)
> +{
> +struct romfile_s *f = romfile_find("etc/msr_feature_control");
> +if (!f)
> +return;
> +
> +u64 feature_control_bits;
> +f->copy(f, &feature_control_bits, sizeof(feature_control_bits));
> +if (feature_control_bits)
> +wrmsr_smp(MSR_IA32_FEATURE_CONTROL, feature_control_bits);
> +}

Can you use romfile_loadint() instead?  Something like:

u64 feature_control_bits = romfile_loadint("etc/msr_feature_control", 0);
if (feature_control_bits)
wrmsr_smp(MSR_IA32_FEATURE_CONTROL, feature_control_bits);

That should avoid the need for the separate romfile_find() call and it
has the added benefit of additional sanity checks.

Also, I think this code is small enough it can be placed directly in
paravirt.c and does not need its own c file.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] UEFI project ideas

[Kevin O'Connor]

On Mon, Jun 06, 2016 at 10:05:31PM +0200, Rudolf Marek wrote:
> Hi all,
> 
> I noticed that seabios/libpayload could have interresting use cases and I want
> to share/discuss them.
> 
> 1) Have a SeaBIOS be a UEFI application. This would benefit on UEFI platforms
> without CSM.

That would be difficult as SeaBIOS really wants to live at 0xf.
Outside of CSM I don't think that's possible on UEFI.

> 2) Provide a minimum UEFI environment. As I noticed u-boot started to have 
> such
> support. In fact it has UEFI glue to u-boot drivers. As such it provides a
> minimal boot services + minimum runtime services. In seabios, it is almost
> there, only PE loader and filesystem support is missing.

Are you suggesting that SeaBIOS be able to load and run UEFI operating
systems without tianocore (or similar)?  (That is, implement a minimal
uefi spec natively within SeaBIOS?)  I looked at that briefly a few
years back, but the uefi spec is crazy complex.  It may be feasible if
someone (eg, uboot) has been able to identify a minimal implementation
that works in practice with modern real world OSes.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH]:splash: Skip the RGB555 mode

[Kevin O'Connor]

On Fri, May 20, 2016 at 03:26:32PM +, Zheng Bao wrote:
> Current JPEG decoding uses the RGB888 or RGB565. So we need to skip
> RGB555 mode.

Thanks.  I updated the indentation and committed this patch.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] keyboard: Inject serial port input as keypresses

[Kevin O'Connor]

On Fri, May 20, 2016 at 02:31:09PM +0300, Kyösti Mälkki wrote:
> Maps characters received from serial port as INT16h keypresses.
> This enables making choice of boot media over serial port.
> 
> Source extracted from SageBIOS release for PC Engines apu1.
> af6c8ab3b85d1a5a9fbeb41efa30a1ef  pcengines.apu_139_osp.tar.gz

Is there a reason to do this in SeaBIOS versus just using sgabios?
The sgabios code seems to have more features (eg, vga output over
serial port).

See some additional comments below.

[...]
> --- a/src/Kconfig
> +++ b/src/Kconfig
> @@ -411,6 +411,12 @@ menu "BIOS interfaces"
>  default y
>  help
>  Support int 16 keyboard calls.
> +config INT16_SERIAL_KEYBOARD
> +bool "Keyboard on serial port"
> +depends on SERIAL && KEYBOARD
> +default n
> +help
> +Translate serial port input to keyboard scancodes.

This would also need to depend on CONFIG_DEBUG_SERIAL (as the code
makes references to CONFIG_DEBUG_SERIAL_PORT), so should probably be
in the debug menu.

[...]
> --- a/src/clock.c
> +++ b/src/clock.c
> @@ -295,6 +295,10 @@ clock_update(void)
>  floppy_tick();
>  usb_check_event();
>  ps2_check_event();
> +
> +#if CONFIG_INT16_SERIAL_KEYBOARD
> +uart_check_keystrokes();
> +#endif

SeaBIOS avoids #if in code - it should read as "if (CONFIG_xyz)
func()".

[...]
> --- a/src/serial.c
> +++ b/src/serial.c
> @@ -315,3 +315,158 @@ handle_17(struct bregs *regs)
>  default:   handle_17XX(regs); break;
>  }
>  }
> +
> +#if CONFIG_INT16_SERIAL_KEYBOARD
> +static u8 UartToScanCode[] VAR16 = {
> +//x0x1x2x3x4x5x6x7x8x9xaxb   
>  xcxdxexf
> +//  
> =
> +0x0f, 0x1e, 0x30, 0x2e, 0x20, 0x12, 0x21, 0x22, 0x23, 0x17, 0x24, 0x25, 
> 0x26, 0x32, 0x31, 0x18, // 0x

To fit with SeaBIOS style, the code should be limited to 80 characters
in length.

[...]
> +void
> +uart_check_keystrokes(void)
> +{
> +u8 rx_buf[5], rx_bytes = 0, ascii_code = 0, scan_code = 0;
> +
> +// check to see if there is a active serial port
> +if (inb(CONFIG_DEBUG_SERIAL_PORT + SEROFF_LSR) == 0xFF)
> +return;
> +
> +while (inb(CONFIG_DEBUG_SERIAL_PORT + SEROFF_LSR) & 0x01) {
> +if (rx_bytes > sizeof(rx_buf)) {
> +dprintf(1, "uart_check_keystrokes: error too many bytes are 
> available\n");
> +while (inb(CONFIG_DEBUG_SERIAL_PORT + SEROFF_LSR) & 0x01)
> +inb(CONFIG_DEBUG_SERIAL_PORT + SEROFF_DATA);
> +return;
> +   }
> +   else
> +   rx_buf[rx_bytes++] = inb(CONFIG_DEBUG_SERIAL_PORT + SEROFF_DATA);
> +}
> +
> +if (!rx_bytes)
> +return;
> +
> +if (rx_bytes == 1) {

This does not look correct to me - the number of serial port
characters read at one time is likely to be pretty random - it's going
to depend on the speed of the serial port vs the speed of the timer
interrupt that polls the port.  So, this seems like there is a good
chance it would lead to some vt100 sequences not being interpretted
and to some regular keys being dropped due to being misinterpreted as
extended sequences.

FWIW, the sgabios code seems to handle the above correctly.

[...]
> +}
> +else if (rx_bytes == 2) { // assume it's actually 2 single-byte 
> keystrokes

To keep with SeaBIOS style, "else" statements should be put on the
same line as the preceeding curly bracket.

Thanks,
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 1/2] scripts/acpi_*: use env python instead of python in shebang

[Kevin O'Connor]

On Thu, May 19, 2016 at 02:25:19AM +0200, Alexander Couzens wrote:
> python should be called via env to support virtualenvs

Is this a general cleanup, or did you run into a specific problem?
These scripts are typically run from the makefile which runs them with
the interpreter specified: $(PYTHON) scrits/acpi_extract.py ...

Thanks,
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 2/2] use python2 instead of python

[Kevin O'Connor]

On Thu, May 19, 2016 at 02:25:20AM +0200, Alexander Couzens wrote:
> python is ambiguous. it could mean python2 or python3.

The scripts were written to run with either python2 or python3.  Did
you run into a compatibility issue?

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2 0/4] support booting more than 255 CPUs with QEMU

[Kevin O'Connor]

On Wed, May 11, 2016 at 12:03:37PM +0200, Igor Mammedov wrote:
> Changelog since:
>   v1:
> * s/count_cpu/apic_id_init/
> * merge handle_x2apic() into apic_id_init()
>   RFC:
> * move out max-cpus check out of mptable_setup()
> * factor out CPU counting/apic ID detection in separate function
> * return back accidentially deleted debug message with APIC ID
> * drop unused code in smp_setup()
> 
> According to SDM, if CPUs have APIC ID more than 254
> firmware should pass control to OS in x2APIC mode.
> This series adds x2APIC bootstrap initialization.
> 
> QEMU side of x2APIC support:
> https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg01094.html

Thanks Igor.  The first 3 patches look good to me.  I'll commit to
SeaBIOS once the corresponding QEMU support is accepted.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] tcgbios: Remove unused const variable

[Kevin O'Connor]

On Fri, May 13, 2016 at 07:29:30AM +0200, Paul Menzel wrote:
> Date: Fri, 13 May 2016 07:10:32 +0200
> Subject: [PATCH 2/2] tcgbios: Remove unused const variable
> 
> Remove the unused array `PhysicalPresence_CMD_DISABLE` to fix the GCC 6
> warnings below.

Thanks.  Your patches got corrupted, but I applied them manually.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC PATCH v4] fw/pci: Add support for mapping Intel IGD via QEMU

[Kevin O'Connor]

On Tue, May 10, 2016 at 09:19:52AM -0600, Alex Williamson wrote:
> On Tue, 23 Feb 2016 10:22:33 -0500
> "Kevin O'Connor"  wrote:
> 
> > On Tue, Feb 16, 2016 at 02:39:27PM -0700, Alex Williamson wrote:
> > > QEMU provides two fw_cfg files to support IGD.  The first holds the
> > > OpRegion data which holds the Video BIOS Table (VBT).  This needs to
> > > be copied into reserved memory and the address stored in the ASL
> > > Storage register of the device at 0xFC offset in PCI config space.
> > > The OpRegion is generally 8KB.  This file is named "etc/igd-opregion".
> > > 
> > > The second file tells us the required size of the stolen memory space
> > > for the device.  This is a dummy file, it has no backing so we only
> > > allocate the space without copying anything into it.  This space
> > > requires 1MB alignment and is generally either 1MB or 2MB, depending
> > > on the hardware config.  If the user has opted in QEMU to expose
> > > additional stolen memory beyond the GTT (GGMS), the GMS may add an
> > > additional 32MB to 512MB.  The base address of the reserved memory
> > > allocated for this is written back to the Base Data of Stolen Memory
> > > register (BDSM) at PCI config offset 0x5C on the device.  This file is
> > > named "etc/igd-bdsm".  
> > 
> > Thanks.  I'd be interested in seeing how the discussion with Michael
> > and Igor works out (wrt using a standardized interface for
> > communicating addresses between qemu and firmware) before adopting
> > this interface in SeaBIOS.
> 
> Apologies if I missed it, but I haven't seen any work in this
> direction, please point me to it if I'm wrong.  Now that QEMU 2.6 is
> wrapping up, I'd like to get IGD assignment support in for 2.7, but we
> can't do that without BIOS support.  How shall we proceed?  Thanks,

If there's no consensus on standardizing the qemu/firmware interface
for addresses then I'm okay with the custom solution you last
proposed.  So, I'd say submit your patches and if they are accepted in
QEMU then I will follow suit and apply them to SeaBIOS.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 0/4] support booting more than 255 CPUs with QEMU

[Kevin O'Connor]

On Wed, May 11, 2016 at 12:02:40PM +0200, Igor Mammedov wrote:
> On Tue, 10 May 2016 16:43:30 +0200
> Igor Mammedov  wrote:
> 
> Probably question to Kevin,
> I've tried to make AP bootstrap run in parallel
> to speed up smp_setup with many CPUs.
> However it hangs and I couldn't debug it.
> Debugging works fine in 16bit mode as described in wiki
> but from the point when entry_smp() switches to 32bit mode
> I don't know how to/where/where from load symbols for
> 32-bit code, even 'disassemble $ip,+xxx' in gdb doesn't
> work as expected.
> Perhaps the debugging section in wiki needs to be updated
> to show how to debug seabios when it's in 32 mode.

Did you disable CONFIG_RELOCATE_INIT as described in the wiki?  (The
smp.c code is considered part of the "init" portion of SeaBIOS.)
Otherwise, the addresses should just work in 32bit mode.

As for threading the smp init - the code currently takes an atomic
lock to control access to a shared stack.  That would need to change
in order to support simultaneous smp init.  Also note that the core
code, such as dprintf, doesn't support true smp multi-threading.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 4/4] cleanup smp_setup()

[Kevin O'Connor]

On Wed, May 11, 2016 at 11:50:36AM +0200, Igor Mammedov wrote:
> On Tue, 10 May 2016 16:43:34 +0200
> Igor Mammedov  wrote:
> 
> > MaxCountCPUs could never be 0 or less CountCPUs
> > anymore, remove code that wouldn't be executed.
> > 
> > Signed-off-by: Igor Mammedov 
> > ---
> >  src/fw/smp.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/src/fw/smp.c b/src/fw/smp.c
> > index 18a4c77..4d82502 100644
> > --- a/src/fw/smp.c
> > +++ b/src/fw/smp.c
> > @@ -175,8 +175,6 @@ smp_setup(void)
> >  *(u64*)BUILD_AP_BOOT_ADDR = old;
> Also looking at usage of BUILD_AP_BOOT_ADDR, it doesn't seem
> to user anywhere beside of smp_setup() and
> smp_setup every time overwrites it to point to entry_smp trampoline
> So saving/restoring 'old' value doesn't look like necessary,
> should it be cleaned up as well?

That's in there because technically malloc_tmplow() could give out
that address range (see alloc_add(&ZoneTmpLow) in malloc_preinit).
So, the idea is to save/restore the area so that the trampoline
doesn't alter anything stored there.  In practice nothing that early
in the boot would use enough malloc_tmplow space to give out that
address, but it doesn't hurt to save/restore either.


> >  handle_x2apic();
> > -if (!MaxCountCPUs || MaxCountCPUs < CountCPUs)
> > -MaxCountCPUs = CountCPUs;

BTW, the "MaxCountCPUs < CountCPUs" check was in there for really old
versions of QEMU that didn't populate "etc/max-cpus".  Unless you're
sure that "etc/max-cpus" predates CMOS_BIOS_SMP_COUNT, I'd be inclined
to leave the check in there.  (No need to resend the series if not
sure).

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 3/4] support booting with more than 255 CPUs

[Kevin O'Connor]

On Tue, May 10, 2016 at 05:31:33PM +0200, Igor Mammedov wrote:
> On Tue, 10 May 2016 11:20:54 -0400
> "Kevin O'Connor"  wrote:
> 
> > On Tue, May 10, 2016 at 04:43:33PM +0200, Igor Mammedov wrote:
> > > SDM[*1] says that if there are CPUs with APIC ID
> > > greater than 254, BIOS is to pass control to OS
> > > in x2APIC mode. Use the fact that QEMU passes in
> > > "etc/max-cpus" max possible "APIC ID + 1" to
> > > detect need for x2APIC mode. Also instead of
> > > CMOS_BIOS_SMP_COUNT which is limited to 256 CPUs
> > > use a new rom file "etc/boot-cpus" that QEMU
> > > supporting more than 256 CPUs will provide.
> > > 
> > > *1) SDM: Volume 3: EXTENDED XAPIC (X2APIC):
> > >  Initialization by System Software
> > > 
> > > Signed-off-by: Igor Mammedov 
> > > ---
> > >  src/fw/smp.c | 40 ++--
> > >  src/x86.h|  1 +
> > >  2 files changed, 35 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/src/fw/smp.c b/src/fw/smp.c
> > > index ad98a5f..18a4c77 100644
> > > --- a/src/fw/smp.c
> > > +++ b/src/fw/smp.c
> > > @@ -19,6 +19,9 @@
> > >  #define APIC_LINT1   ((u8*)BUILD_APIC_ADDR + 0x360)
> > >  
> > >  #define APIC_ENABLED 0x0100
> > > +#define MSR_IA32_APIC_BASE 0x01B
> > > +#define MSR_LOCAL_APIC_ID 0x802
> > > +#define MSR_IA32_APICBASE_EXTD (1ULL << 10) /* Enable x2APIC mode */
> > >  
> > >  static struct { u32 index; u64 val; } smp_mtrr[32];
> > >  static u32 smp_mtrr_count;
> > > @@ -49,23 +52,46 @@ int apic_id_is_present(u8 apic_id)
> > >  static int
> > >  count_cpu(void)
> > >  {
> > > +u32 apic_id;
> > >  CountCPUs++;
> > >  
> > > -// Track found apic id for use in legacy internal bios tables
> > >  u32 eax, ebx, ecx, cpuid_features;
> > >  cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > > -u8 apic_id = ebx>>24;
> > > -FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> > > +apic_id = ebx>>24;
> > > +if (MaxCountCPUs < 256) { // xAPIC mode
> > > +// Track found apic id for use in legacy internal bios tables
> > > +FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> > > +}
> > > +if (rdmsr(MSR_IA32_APIC_BASE) & MSR_IA32_APICBASE_EXTD) { // x2APIC 
> > > mode
> > > +apic_id = rdmsr(MSR_LOCAL_APIC_ID);
> > > +}
> > >  
> > >  return apic_id;
> > >  }
> > >  
> > > +static void
> > > +handle_x2apic(void)
> > > +{
> > > +if (MaxCountCPUs < 256)
> > > +return;
> > > +
> > > +u32 eax, ebx, ecx, edx;
> > > +cpuid(1, &eax, &ebx, &ecx, &edx);
> > > +if (!(ecx & CPUID_X2APIC))
> > > +return;
> > > +
> > > +// switch to x2APIC mode
> > > +u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
> > > +wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
> > > +}  
> > 
> > Can we integrate handle_x2apic() into count_cpu() as in:
> > 
> > static int
> > count_cpu(void)
> > {
> > CountCPUs++;
> > 
> > u32 eax, ebx, ecx, cpuid_features;
> > cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> > if (MaxCountCPUs < 256) { // xAPIC mode
> > // Track found apic id for use in legacy internal bios tables
> > FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> > return ebx>>24;
> > }
> > 
> > // x2APIC mode
> > if (!(ecx & CPUID_X2APIC))
> > return -1;
> > u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
> > wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
> > return rdmsr(MSR_LOCAL_APIC_ID);
> > }
> > 
> > count_cpu may be a poor choice of name, but if we could put all the
> > apic stuff in one function and call it from both handle_smp() and
> > smp_setup() I think it would make the code a little easier to
> > understand.
> sure
> 
> perhaps 's/count_cpu/apic_id_init/'

Great.  Thanks.
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 3/4] support booting with more than 255 CPUs

[Kevin O'Connor]

On Tue, May 10, 2016 at 04:43:33PM +0200, Igor Mammedov wrote:
> SDM[*1] says that if there are CPUs with APIC ID
> greater than 254, BIOS is to pass control to OS
> in x2APIC mode. Use the fact that QEMU passes in
> "etc/max-cpus" max possible "APIC ID + 1" to
> detect need for x2APIC mode. Also instead of
> CMOS_BIOS_SMP_COUNT which is limited to 256 CPUs
> use a new rom file "etc/boot-cpus" that QEMU
> supporting more than 256 CPUs will provide.
> 
> *1) SDM: Volume 3: EXTENDED XAPIC (X2APIC):
>  Initialization by System Software
> 
> Signed-off-by: Igor Mammedov 
> ---
>  src/fw/smp.c | 40 ++--
>  src/x86.h|  1 +
>  2 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/src/fw/smp.c b/src/fw/smp.c
> index ad98a5f..18a4c77 100644
> --- a/src/fw/smp.c
> +++ b/src/fw/smp.c
> @@ -19,6 +19,9 @@
>  #define APIC_LINT1   ((u8*)BUILD_APIC_ADDR + 0x360)
>  
>  #define APIC_ENABLED 0x0100
> +#define MSR_IA32_APIC_BASE 0x01B
> +#define MSR_LOCAL_APIC_ID 0x802
> +#define MSR_IA32_APICBASE_EXTD (1ULL << 10) /* Enable x2APIC mode */
>  
>  static struct { u32 index; u64 val; } smp_mtrr[32];
>  static u32 smp_mtrr_count;
> @@ -49,23 +52,46 @@ int apic_id_is_present(u8 apic_id)
>  static int
>  count_cpu(void)
>  {
> +u32 apic_id;
>  CountCPUs++;
>  
> -// Track found apic id for use in legacy internal bios tables
>  u32 eax, ebx, ecx, cpuid_features;
>  cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> -u8 apic_id = ebx>>24;
> -FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> +apic_id = ebx>>24;
> +if (MaxCountCPUs < 256) { // xAPIC mode
> +// Track found apic id for use in legacy internal bios tables
> +FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
> +}
> +if (rdmsr(MSR_IA32_APIC_BASE) & MSR_IA32_APICBASE_EXTD) { // x2APIC mode
> +apic_id = rdmsr(MSR_LOCAL_APIC_ID);
> +}
>  
>  return apic_id;
>  }
>  
> +static void
> +handle_x2apic(void)
> +{
> +if (MaxCountCPUs < 256)
> +return;
> +
> +u32 eax, ebx, ecx, edx;
> +cpuid(1, &eax, &ebx, &ecx, &edx);
> +if (!(ecx & CPUID_X2APIC))
> +return;
> +
> +// switch to x2APIC mode
> +u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
> +wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
> +}

Can we integrate handle_x2apic() into count_cpu() as in:

static int
count_cpu(void)
{
CountCPUs++;

u32 eax, ebx, ecx, cpuid_features;
cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
if (MaxCountCPUs < 256) { // xAPIC mode
// Track found apic id for use in legacy internal bios tables
FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
return ebx>>24;
}

// x2APIC mode
if (!(ecx & CPUID_X2APIC))
return -1;
u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
return rdmsr(MSR_LOCAL_APIC_ID);
}

count_cpu may be a poor choice of name, but if we could put all the
apic stuff in one function and call it from both handle_smp() and
smp_setup() I think it would make the code a little easier to
understand.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH RFC 1/2] paravirt: disable MPTable in case of more than 255 CPUs

[Kevin O'Connor]

On Tue, May 10, 2016 at 02:13:35PM +0200, Igor Mammedov wrote:
> On Mon, 9 May 2016 11:12:25 -0400
> "Kevin O'Connor"  wrote:
> 
> > On Mon, May 09, 2016 at 11:43:53AM +0200, Igor Mammedov wrote:
> > > MPTable doesn't support more than 254 CPUs and
> > > QEMU supplies an alternative MADT table which
> > > guest will use instead of it. So do not install
> > > MPTable if more than 254 CPUs are provided.
> > > 
> > > Signed-off-by: Igor Mammedov 
> > > ---
> > >  src/fw/mptable.c | 4 
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/src/fw/mptable.c b/src/fw/mptable.c
> > > index 47385cc..aec26f8 100644
> > > --- a/src/fw/mptable.c
> > > +++ b/src/fw/mptable.c
> > > @@ -24,6 +24,10 @@ mptable_setup(void)
> > >  if (! CONFIG_MPTABLE)
> > >  return;
> > >  
> > > +if (romfile_loadint("etc/max-cpus", 0) > 255) {
> > > +dprintf(1, "MPTable doesn't support more than 254 CPUs. Skip 
> > > it.\n");
> > > +return;
> > > +}
> > >  dprintf(3, "init MPTable\n");
> > >  
> > >  // Config structure in temp area.  
> > 
> > Thanks.  I'd prefer to not make further modifications to fw/mptable.c
> > though.  I think it would be better to do something like (totally
> > untested):
> > 
> > --- a/src/fw/paravirt.c
> > +++ b/src/fw/paravirt.c
> > @@ -154,8 +154,14 @@ qemu_platform_setup(void)
> >  smp_setup();
> >  
> >  // Create bios tables
> > -pirtable_setup();
> > -mptable_setup();
> > +if (romfile_find("etc/legacy-tables-loader")) {
> > +// QEMU may specify the PIR and mptable (or leave empty for no 
> > tables)
> > +romfile_loader_execute("etc/legacy-tables-loader");
> > +} else {
> > +// Load the legacy internal tables
> > +pirtable_setup();
> > +mptable_setup();
> > +}
> >  smbios_setup();
> >  
> >  if (CONFIG_FW_ROMFILE_LOAD) {
> > 
> > And then QEMU can create an empty fw_cfg file to suppress both the
> > mptable and the pir table.  (Or, it can populate it to specify exactly
> > what QEMU wants in those tables.)
> I don't think QEMU would ever want to provide legacy tables
> as Seabios creates them just fine for supported by them
> amount of CPUs. It's just that mptable_setup() is broken
> when there is more than 255 CPUs which leads to out of bound
> read/write accesses when dealing with APIC ID > 255.
> 
> Considering QEMU not being interested in legacy tables,
> how about following:
> 
> 
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -154,8 +154,10 @@ qemu_platform_setup(void)
>  smp_setup();
>  
>  // Create bios tables
> -pirtable_setup();
> -mptable_setup();
> +if (MaxCountCPUs <= 255) {
> +pirtable_setup();
> +mptable_setup();
> +}
>  smbios_setup();
>  
>  if (CONFIG_FW_ROMFILE_LOAD) {

Okay.  I think something like:

...
if (romfile_loadint("etc/create-legacy-tables", 1)) {
pirtable_setup();
mptable_setup();
}
...

would be better.  But, I'm also okay with the above MaxCountCPUs
solution.

Thanks.
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH RFC 2/2] support booting with more than 255 CPUs

[Kevin O'Connor]

On Mon, May 09, 2016 at 11:43:54AM +0200, Igor Mammedov wrote:
> SDM[*1] says that if there are CPUs with APIC ID
> greater than 254, BIOS is to pass control to OS
> in x2APIC mode. Use the fact that QEMU passes in
> "etc/max-cpus" max possible "APIC ID + 1" to
> detect need for x2APIC mode. Also instead of
> CMOS_BIOS_SMP_COUNT which is limited to 256 CPUs
> use a new rom file "etc/boot-cpus" that QEMU
> supporting more than 256 CPUs will provide.
> 
> *1) SDM: Volume 3: EXTENDED XAPIC (X2APIC):
>  Initialization by System Software
> 
> Signed-off-by: Igor Mammedov 
> ---
>  src/fw/smp.c | 48 +++-
>  src/x86.h|  1 +
>  2 files changed, 40 insertions(+), 9 deletions(-)
> 
> diff --git a/src/fw/smp.c b/src/fw/smp.c
> index 579acdb..2bb5e1b 100644
> --- a/src/fw/smp.c
> +++ b/src/fw/smp.c
> @@ -19,6 +19,9 @@
>  #define APIC_LINT1   ((u8*)BUILD_APIC_ADDR + 0x360)
>  
>  #define APIC_ENABLED 0x0100
> +#define MSR_IA32_APIC_BASE 0x01B
> +#define MSR_LOCAL_APIC_ID 0x802
> +#define MSR_IA32_APICBASE_EXTD (1ULL << 10) /* Enable x2APIC mode */
>  
>  static struct { u32 index; u64 val; } smp_mtrr[32];
>  static u32 smp_mtrr_count;
> @@ -46,6 +49,19 @@ int apic_id_is_present(u8 apic_id)
>  return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32)));
>  }
>  
> +static void handle_x2apic(u32 has_x2apic)
> +{
> +if (MaxCountCPUs < 256)
> +return;
> +
> +if (!has_x2apic)
> +return;
> +
> +// switch to x2APIC mode
> +u64 apic_base = rdmsr(MSR_IA32_APIC_BASE);
> +wrmsr(MSR_IA32_APIC_BASE, apic_base | MSR_IA32_APICBASE_EXTD);
> +}
> +
>  void VISIBLE32FLAT
>  handle_smp(void)
>  {
> @@ -55,17 +71,24 @@ handle_smp(void)
>  // Detect apic_id
>  u32 eax, ebx, ecx, cpuid_features;
>  cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
> -u8 apic_id = ebx>>24;
> -dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);

Is it no longer possible to report an identifier for the apic?  Was
the dprintf removed because the log was filled with cpu reports or
because there is no equivalent id?

> +
> +handle_x2apic(ecx & CPUID_X2APIC);
>  
>  // MTRR setup
>  int i;
>  for (i=0; i  wrmsr(smp_mtrr[i].index, smp_mtrr[i].val);
>  
> -// Set bit on FoundAPICIDs
> -FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> -
> +/*
> + * QEMU that supports APIC ID > 255 provides its own BIOS tables
> + * so skip filling present APIC map as it's not used.
> + * (it's used for internal BIOS tables for QEMU older than 1.8)
> + */
> +if (MaxCountCPUs < 255) {
> +   u32 apic_id = ebx>>24;
> +   // Set bit on FoundAPICIDs
> +   FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
> +}
>  CountCPUs++;
>  }
>  
> @@ -91,6 +114,11 @@ smp_setup(void)
>  return;
>  }
>  
> +/* set max possible APIC ID limit for AP bootstrap to decide
> + * if it neds to switch into x2APIC mode
> + */
> +MaxCountCPUs = romfile_loadint("etc/max-cpus", 1);
> +
>  // mark the BSP initial APIC ID as found, too:
>  u8 apic_id = ebx>>24;
>  FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));

This updates FoundAPICIDs even if MaxCountCPUs > 255 which is a little
confusing.

I think this patch would be simpler if the updating of FoundAPICIDs
was refactored first.  Something like the below (totally untested).

-Kevin


--- a/src/fw/smp.c
+++ b/src/fw/smp.c
@@ -46,27 +46,34 @@ int apic_id_is_present(u8 apic_id)
 return !!(FoundAPICIDs[apic_id/32] & (1ul << (apic_id % 32)));
 }
 
+static int
+count_cpu(void)
+{
+CountCPUs++;
+
+// Track found apic id for use in legacy internal bios tables
+u32 eax, ebx, ecx, cpuid_features;
+cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
+u8 apic_id = ebx>>24;
+FoundAPICIDs[apic_id/32] |= 1 << (apic_id % 32);
+
+return apic_id;
+}
+
 void VISIBLE32FLAT
 handle_smp(void)
 {
 if (!CONFIG_QEMU)
 return;
 
-// Detect apic_id
-u32 eax, ebx, ecx, cpuid_features;
-cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
-u8 apic_id = ebx>>24;
+// Track this CPU and detect the apic_id
+int apic_id = count_cpu();
 dprintf(DEBUG_HDL_smp, "handle_smp: apic_id=%d\n", apic_id);
 
 // MTRR setup
 int i;
 for (i=0; i>24;
-FoundAPICIDs[apic_id/32] |= (1 << (apic_id % 32));
-CountCPUs = 1;
+// Detect initial boot cpu
+count_cpu();
 
 // Setup jump trampoline to counter code.
 u64 old = *(u64*)BUILD_AP_BOOT_ADDR;

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH RFC 1/2] paravirt: disable MPTable in case of more than 255 CPUs

[Kevin O'Connor]

On Mon, May 09, 2016 at 11:43:53AM +0200, Igor Mammedov wrote:
> MPTable doesn't support more than 254 CPUs and
> QEMU supplies an alternative MADT table which
> guest will use instead of it. So do not install
> MPTable if more than 254 CPUs are provided.
> 
> Signed-off-by: Igor Mammedov 
> ---
>  src/fw/mptable.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/fw/mptable.c b/src/fw/mptable.c
> index 47385cc..aec26f8 100644
> --- a/src/fw/mptable.c
> +++ b/src/fw/mptable.c
> @@ -24,6 +24,10 @@ mptable_setup(void)
>  if (! CONFIG_MPTABLE)
>  return;
>  
> +if (romfile_loadint("etc/max-cpus", 0) > 255) {
> +dprintf(1, "MPTable doesn't support more than 254 CPUs. Skip it.\n");
> +return;
> +}
>  dprintf(3, "init MPTable\n");
>  
>  // Config structure in temp area.

Thanks.  I'd prefer to not make further modifications to fw/mptable.c
though.  I think it would be better to do something like (totally
untested):

--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -154,8 +154,14 @@ qemu_platform_setup(void)
 smp_setup();
 
 // Create bios tables
-pirtable_setup();
-mptable_setup();
+if (romfile_find("etc/legacy-tables-loader")) {
+// QEMU may specify the PIR and mptable (or leave empty for no tables)
+romfile_loader_execute("etc/legacy-tables-loader");
+} else {
+// Load the legacy internal tables
+pirtable_setup();
+mptable_setup();
+}
 smbios_setup();
 
 if (CONFIG_FW_ROMFILE_LOAD) {

And then QEMU can create an empty fw_cfg file to suppress both the
mptable and the pir table.  (Or, it can populate it to specify exactly
what QEMU wants in those tables.)

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] qemu-kvm: SeaBIOS 1.9.0 and above cannot boot Windows 10 from harddisk

[Kevin O'Connor]

On Tue, Apr 26, 2016 at 11:16:20PM +0200, Johannes Krottmayer wrote:
> Am 2016-04-26 um 22:38 schrieb Kevin O'Connor:
> [...]
> > To report a SeaBIOS issue, the SeaBIOS log is needed.  Please see
> > http://www.seabios.org/Debugging#Diagnostic_information
> 
> $ qemu-system-x86_64 -chardev stdio,id=seabios -device
> isa-debugcon,iobase=0x402,chardev=seabios -bios
> ../qemu/seabios-1.9.2/out/bios.bin -machine pc,accel=kvm -cpu core2duo
> -m 2048 -smp 2 -hda win10.img -boot c
> SeaBIOS (version 1.9.2-20160426_212753-zeus)
[...]
> Booting from Hard Disk...
> Booting from :7c00
> KVM internal error. Suberror: 1
> emulation failure

It appears the fault is occurring in the OS bootloader, not in the
SeaBIOS code.

[...]
> >> QEMU output with SeaBIOS 1.9.0 and above:
> > 
> > Did it work with some prior version of SeaBIOS?
> 
> Yes. Tested it with the default version (1.7.2.2) of my distribution,
> 1.8.0, 1.8.1 and version 1.8.2.

It's odd that it works with a different SeaBIOS version.  You could
try bisecting between 1.8.0 and 1.9.2 to see what change the crash
starts at - but be aware that random differences in the SeaBIOS binary
might be tickling the underlying issue.

You could also try reporting to the kvm list - they'll know how to
interpret the cpu dump.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] qemu-kvm: SeaBIOS 1.9.0 and above cannot boot Windows 10 from harddisk

[Kevin O'Connor]

On Tue, Apr 26, 2016 at 10:10:04PM +0200, Johannes Krottmayer wrote:
> I have following problem...

To report a SeaBIOS issue, the SeaBIOS log is needed.  Please see
http://www.seabios.org/Debugging#Diagnostic_information

[...]
> QEMU output with SeaBIOS 1.9.0 and above:

Did it work with some prior version of SeaBIOS?

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 2/2] shadow: Batch PCI config writes

[Kevin O'Connor]

On Tue, Apr 05, 2016 at 12:51:56PM -0400, Kevin O'Connor wrote:
> Enabling and disabling shadow ram on QEMU is slow.  Batch the PCI
> writes to reduce the number of memory changes QEMU must implement.

FYI, I committed this change.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 2/2] scsi: Launch a thread when scanning for drives in the scsi drivers

[Kevin O'Connor]

On Tue, Apr 05, 2016 at 01:06:32PM -0400, Kevin O'Connor wrote:
> Signed-off-by: Kevin O'Connor 

FYI, I committed this change.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 2/2] disk: Avoid stack_hop() path if already on the extra stack

[Kevin O'Connor]

On Thu, Mar 31, 2016 at 02:50:32PM -0400, Kevin O'Connor wrote:
> If CONFIG_ENTRY_EXTRASTACK is set (enabled by default) then the 16bit
> disk interface code is already running on the extra stack and it is
> not necessary to support stack switching on each disk request.

FYI, I committed this change.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] Add support for legacy USB keyboard/mouse (PS/2 emulation for USB?)

[Kevin O'Connor]

On Sat, Apr 16, 2016 at 01:26:43AM +0200, Adam Rutkowski wrote:
> Hello,
> 
> I think this is the right place to write about this. From a few days
> I'm fighting with not USB-aware OS-es in QEMU (like Windows 98, IBM
> OS/2, MS-DOS etc.). When I assign real USB keyboard and mouse to VM,
> keyboard partially works in DOS (keys like Alt aren't working), but
> mouse doesn't work at all.

In my quick tests on freedos, both the usb keyboard alt key and usb
mouse work.  That said, I don't doubt old DOS device drivers and old
OSes (eg, win98) would attempt to directly access the ps2 hardware and
thus not benefit from the SeaBIOS USB drivers.

>After starting, let's say Windows 98
> Setup, keyboard stops working too. From my experience these
> peripherals works only in newer OSes like Windows 2000+ or Linux,
> because they initialize their own USB HID driver. I'm not using
> QEMU's emulated USB HID devices, because I utilize VGA passthrough,
> so I need direct access from guest machine.
> 
> Summarizing: is it possible to add full support for USB keyboard and
> mouse in SeaBIOS?

Unfortunately, emulating a ps2 keyboard/mouse in SeaBIOS is very
difficult, and I don't think it's practical.

However, it should be easy to have QEMU emulate a ps2 keyboard/mouse.
It should also be possible to do that emulation even if the VGA is in
passthrough.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC] Document and rework debug levels

[Kevin O'Connor]

On Wed, Apr 06, 2016 at 07:58:28PM +0200, Laszlo Ersek wrote:
> On 04/06/16 19:52, Kevin O'Connor wrote:
> > On Wed, Apr 06, 2016 at 07:38:49PM +0200, Laszlo Ersek wrote:
> >> On 04/06/16 19:07, Kevin O'Connor wrote:
> >>> There are a number of debug levels in the SeaBIOS code today.
> >>> Unfortunately, much of the code does not follow any particular
> >>> standard for which debug level to use.
> >>>
> >>> This is becoming cumbersome for a few reasons:
> >>>
> >>> - some people want fewer debug messages to reduce boot time, but still
> >>>   want critical messages (eg, error messages).  Debug level 1 can be
> >>>   too verbose while debug level 0 disables all messages.
> >>>
> >>> - some drivers become very verbose before other drivers, and thus
> >>>   depending on a user's hardware, the user might get flooded with
> >>>   messages they don't want before seeing the important messages from
> >>>   the driver they were looking for
> >>>
> >>> - when writing a new driver, it's not easy for a developer to know
> >>>   what debug level to choose
> >>>
> >>> I'm proposing that the debug levels be documented and that all of the
> >>> debug messages in SeaBIOS be reworked to follow that documentation.
> >>> Here are the debug levels I propose using:
> >>>
> >>> Level 1:
> >>>   - SeaBIOS version banner
> >>>   - Major error messages and major warning messages that are likely
> >>> indicative of an error
> >>>   - Screen output (ie, printf) would also be available at this level
> >>> when screen-and-debug is true
> >>> Level 2:
> >>>   - Critical memory layout information.  This would include the e820
> >>> map prior to booting and similar information about UMB memory,
> >>> EBDA memory, BIOS table locations, etc.
> >>> Level 3:
> >>>   - Found hardware for which there is a SeaBIOS driver and critical
> >>> information about that hardware (such as its address, hardware
> >>> version, hardware capabilities, etc.)
> >>> Level 4:
> >>>   - Major code flow events between SeaBIOS phases (eg, post, boot,
> >>> resume phases)
> >>> Level 5:
> >>>   - Code flow notifications at driver and subsystem startup (eg, "init
> >>> usb\n" type messages)
> >>>   - Thread startup and shutdown messages
> >>> Level 6:
> >>>   - Basic driver and subsystem debugging.  This would be driver
> >>> specific messages that are not expected to be called with high
> >>> frequency (ie, not called on every disk access nor on every
> >>> keyboard access, etc.)
> >>>
> >>> Finally, for debug messages that could be called with high frequency
> >>> (eg, on each disk read), I propose defining a DEBUG_xyz at the top of
> >>> the particular driver source code file which would default to 9 and a
> >>> developer could change to a lower number when working on that code.
> >>> So, for example, DEBUG_xhci could be introduced and be used on
> >>> dprintf() messages that are invoked on each xhci transfer.
> >>>
> >>> As part of this proposal, the default debug level would change from
> >>> level 1 to level 4.  Most dprintf level 1 calls in the code would
> >>> change to a level between 1 and 4.  Most driver dprintf calls between
> >>> levels 3-8 would end up changing to level 6.  Most dprintf calls with
> >>> 9 (or higher) would instead get a DEBUG_xyz definition.
> >>>
> >>> Thoughts?
> >>
> >> Seems well-thought-out to me. Any chance you could introduce symbolic
> >> constants too for the debug levels?
> > 
> > It would be nice to do that.  However, I'm not sure if:
> > 
> > dprintf(DEBUG_memory, "my message\n");
> > 
> > would become too cumbersome due to the length of the symbol names.
> 
> For one data point, it wouldn't bother me. (I'm used to this pattern --
> in edk2 the debug log level is a log mask actually. Occasionally a
> bitmask (of two bits) is constructed on the spot.)
> 
> On the other hand:
> 
> > Maybe adding wrappers (eg, debug_mem("my message") ) for the common
> > cases would avoid that problem.
> 
> would nicely imitate pr_*() from Linux's "include/linux/printk.h".

The printk.h code is interesting.  I'm now thinking it might be better
to introduce pr_err (for level 1 above), pr_notice (for levels 2-4
above), pr_info (for level 5), and pr_debug (for level 6).  The "debug
level" would then just be an internal detail.

It still has the issue of making sure debug levels are used
consistently across the code, but we could just use documentation for
that.

For the infrequent debug messages (existing levels 9+) we could
introduce macros like:

#define pr_debug_xhci(...) // pr_debug(__VA_ARGS__)

and a developer could alter the macro to uncomment the call to
pr_debug() when the additional info is desired.

Thanks,
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [RFC] Document and rework debug levels

[Kevin O'Connor]

On Wed, Apr 06, 2016 at 07:38:49PM +0200, Laszlo Ersek wrote:
> On 04/06/16 19:07, Kevin O'Connor wrote:
> > There are a number of debug levels in the SeaBIOS code today.
> > Unfortunately, much of the code does not follow any particular
> > standard for which debug level to use.
> > 
> > This is becoming cumbersome for a few reasons:
> > 
> > - some people want fewer debug messages to reduce boot time, but still
> >   want critical messages (eg, error messages).  Debug level 1 can be
> >   too verbose while debug level 0 disables all messages.
> > 
> > - some drivers become very verbose before other drivers, and thus
> >   depending on a user's hardware, the user might get flooded with
> >   messages they don't want before seeing the important messages from
> >   the driver they were looking for
> > 
> > - when writing a new driver, it's not easy for a developer to know
> >   what debug level to choose
> > 
> > I'm proposing that the debug levels be documented and that all of the
> > debug messages in SeaBIOS be reworked to follow that documentation.
> > Here are the debug levels I propose using:
> > 
> > Level 1:
> >   - SeaBIOS version banner
> >   - Major error messages and major warning messages that are likely
> > indicative of an error
> >   - Screen output (ie, printf) would also be available at this level
> > when screen-and-debug is true
> > Level 2:
> >   - Critical memory layout information.  This would include the e820
> > map prior to booting and similar information about UMB memory,
> > EBDA memory, BIOS table locations, etc.
> > Level 3:
> >   - Found hardware for which there is a SeaBIOS driver and critical
> > information about that hardware (such as its address, hardware
> > version, hardware capabilities, etc.)
> > Level 4:
> >   - Major code flow events between SeaBIOS phases (eg, post, boot,
> > resume phases)
> > Level 5:
> >   - Code flow notifications at driver and subsystem startup (eg, "init
> > usb\n" type messages)
> >   - Thread startup and shutdown messages
> > Level 6:
> >   - Basic driver and subsystem debugging.  This would be driver
> > specific messages that are not expected to be called with high
> > frequency (ie, not called on every disk access nor on every
> > keyboard access, etc.)
> > 
> > Finally, for debug messages that could be called with high frequency
> > (eg, on each disk read), I propose defining a DEBUG_xyz at the top of
> > the particular driver source code file which would default to 9 and a
> > developer could change to a lower number when working on that code.
> > So, for example, DEBUG_xhci could be introduced and be used on
> > dprintf() messages that are invoked on each xhci transfer.
> > 
> > As part of this proposal, the default debug level would change from
> > level 1 to level 4.  Most dprintf level 1 calls in the code would
> > change to a level between 1 and 4.  Most driver dprintf calls between
> > levels 3-8 would end up changing to level 6.  Most dprintf calls with
> > 9 (or higher) would instead get a DEBUG_xyz definition.
> > 
> > Thoughts?
> 
> Seems well-thought-out to me. Any chance you could introduce symbolic
> constants too for the debug levels?

It would be nice to do that.  However, I'm not sure if:

dprintf(DEBUG_memory, "my message\n");

would become too cumbersome due to the length of the symbol names.
Maybe adding wrappers (eg, debug_mem("my message") ) for the common
cases would avoid that problem.

Thanks,
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [RFC] Document and rework debug levels

[Kevin O'Connor]

There are a number of debug levels in the SeaBIOS code today.
Unfortunately, much of the code does not follow any particular
standard for which debug level to use.

This is becoming cumbersome for a few reasons:

- some people want fewer debug messages to reduce boot time, but still
  want critical messages (eg, error messages).  Debug level 1 can be
  too verbose while debug level 0 disables all messages.

- some drivers become very verbose before other drivers, and thus
  depending on a user's hardware, the user might get flooded with
  messages they don't want before seeing the important messages from
  the driver they were looking for

- when writing a new driver, it's not easy for a developer to know
  what debug level to choose

I'm proposing that the debug levels be documented and that all of the
debug messages in SeaBIOS be reworked to follow that documentation.
Here are the debug levels I propose using:

Level 1:
  - SeaBIOS version banner
  - Major error messages and major warning messages that are likely
indicative of an error
  - Screen output (ie, printf) would also be available at this level
when screen-and-debug is true
Level 2:
  - Critical memory layout information.  This would include the e820
map prior to booting and similar information about UMB memory,
EBDA memory, BIOS table locations, etc.
Level 3:
  - Found hardware for which there is a SeaBIOS driver and critical
information about that hardware (such as its address, hardware
version, hardware capabilities, etc.)
Level 4:
  - Major code flow events between SeaBIOS phases (eg, post, boot,
resume phases)
Level 5:
  - Code flow notifications at driver and subsystem startup (eg, "init
usb\n" type messages)
  - Thread startup and shutdown messages
Level 6:
  - Basic driver and subsystem debugging.  This would be driver
specific messages that are not expected to be called with high
frequency (ie, not called on every disk access nor on every
keyboard access, etc.)

Finally, for debug messages that could be called with high frequency
(eg, on each disk read), I propose defining a DEBUG_xyz at the top of
the particular driver source code file which would default to 9 and a
developer could change to a lower number when working on that code.
So, for example, DEBUG_xhci could be introduced and be used on
dprintf() messages that are invoked on each xhci transfer.

As part of this proposal, the default debug level would change from
level 1 to level 4.  Most dprintf level 1 calls in the code would
change to a level between 1 and 4.  Most driver dprintf calls between
levels 3-8 would end up changing to level 6.  Most dprintf calls with
9 (or higher) would instead get a DEBUG_xyz definition.

Thoughts?
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 1/2] virtio: Use threads when scanning for virtio devices

[Kevin O'Connor]

Signed-off-by: Kevin O'Connor 
---
 src/hw/virtio-blk.c  | 6 --
 src/hw/virtio-scsi.c | 6 --
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c
index 2dfd0c3..dca7855 100644
--- a/src/hw/virtio-blk.c
+++ b/src/hw/virtio-blk.c
@@ -15,6 +15,7 @@
 #include "pcidevice.h" // foreachpci
 #include "pci_ids.h" // PCI_DEVICE_ID_VIRTIO_BLK
 #include "pci_regs.h" // PCI_VENDOR_ID
+#include "stacks.h" // run_thread
 #include "std/disk.h" // DISK_RET_SUCCESS
 #include "string.h" // memset
 #include "util.h" // usleep
@@ -93,8 +94,9 @@ virtio_blk_process_op(struct disk_op_s *op)
 }
 
 static void
-init_virtio_blk(struct pci_device *pci)
+init_virtio_blk(void *data)
 {
+struct pci_device *pci = data;
 u8 status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER;
 dprintf(1, "found virtio-blk at %pP\n", pci);
 struct virtiodrive_s *vdrive = malloc_fseg(sizeof(*vdrive));
@@ -203,6 +205,6 @@ virtio_blk_setup(void)
 (pci->device != PCI_DEVICE_ID_VIRTIO_BLK_09 &&
  pci->device != PCI_DEVICE_ID_VIRTIO_BLK_10))
 continue;
-init_virtio_blk(pci);
+run_thread(init_virtio_blk, pci);
 }
 }
diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c
index 322d469..5fb9409 100644
--- a/src/hw/virtio-scsi.c
+++ b/src/hw/virtio-scsi.c
@@ -16,6 +16,7 @@
 #include "pcidevice.h" // foreachpci
 #include "pci_ids.h" // PCI_DEVICE_ID_VIRTIO_BLK
 #include "pci_regs.h" // PCI_VENDOR_ID
+#include "stacks.h" // run_thread
 #include "std/disk.h" // DISK_RET_SUCCESS
 #include "string.h" // memset
 #include "util.h" // usleep
@@ -132,8 +133,9 @@ virtio_scsi_scan_target(struct pci_device *pci, struct 
vp_device *vp,
 }
 
 static void
-init_virtio_scsi(struct pci_device *pci)
+init_virtio_scsi(void *data)
 {
+struct pci_device *pci = data;
 dprintf(1, "found virtio-scsi at %pP\n", pci);
 struct vring_virtqueue *vq = NULL;
 struct vp_device *vp = malloc_high(sizeof(*vp));
@@ -199,6 +201,6 @@ virtio_scsi_setup(void)
 (pci->device != PCI_DEVICE_ID_VIRTIO_SCSI_09 &&
  pci->device != PCI_DEVICE_ID_VIRTIO_SCSI_10))
 continue;
-init_virtio_scsi(pci);
+run_thread(init_virtio_scsi, pci);
 }
 }
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 2/2] scsi: Launch a thread when scanning for drives in the scsi drivers

[Kevin O'Connor]

Signed-off-by: Kevin O'Connor 
---
 src/hw/esp-scsi.c |  8 
 src/hw/lsi-scsi.c |  8 
 src/hw/megasas.c  |  7 +++
 src/hw/mpt-scsi.c | 23 ++-
 src/hw/pvscsi.c   |  8 
 5 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/src/hw/esp-scsi.c b/src/hw/esp-scsi.c
index c98989c..d2ba023 100644
--- a/src/hw/esp-scsi.c
+++ b/src/hw/esp-scsi.c
@@ -20,6 +20,7 @@
 #include "pcidevice.h" // foreachpci
 #include "pci_ids.h" // PCI_DEVICE_ID
 #include "pci_regs.h" // PCI_VENDOR_ID
+#include "stacks.h" // run_thread
 #include "std/disk.h" // DISK_RET_SUCCESS
 #include "string.h" // memset
 #include "util.h" // usleep
@@ -188,8 +189,9 @@ esp_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 
target)
 }
 
 static void
-init_esp_scsi(struct pci_device *pci)
+init_esp_scsi(void *data)
 {
+struct pci_device *pci = data;
 u32 iobase = pci_enable_iobar(pci, PCI_BASE_ADDRESS_0);
 if (!iobase)
 return;
@@ -203,8 +205,6 @@ init_esp_scsi(struct pci_device *pci)
 int i;
 for (i = 0; i <= 7; i++)
 esp_scsi_scan_target(pci, iobase, i);
-
-return;
 }
 
 void
@@ -221,6 +221,6 @@ esp_scsi_setup(void)
 if (pci->vendor != PCI_VENDOR_ID_AMD
 || pci->device != PCI_DEVICE_ID_AMD_SCSI)
 continue;
-init_esp_scsi(pci);
+run_thread(init_esp_scsi, pci);
 }
 }
diff --git a/src/hw/lsi-scsi.c b/src/hw/lsi-scsi.c
index fd695fa..b63430d 100644
--- a/src/hw/lsi-scsi.c
+++ b/src/hw/lsi-scsi.c
@@ -20,6 +20,7 @@
 #include "pcidevice.h" // foreachpci
 #include "pci_ids.h" // PCI_DEVICE_ID_VIRTIO_BLK
 #include "pci_regs.h" // PCI_VENDOR_ID
+#include "stacks.h" // run_thread
 #include "std/disk.h" // DISK_RET_SUCCESS
 #include "string.h" // memset
 #include "util.h" // usleep
@@ -168,8 +169,9 @@ lsi_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 
target)
 }
 
 static void
-init_lsi_scsi(struct pci_device *pci)
+init_lsi_scsi(void *data)
 {
+struct pci_device *pci = data;
 u32 iobase = pci_enable_iobar(pci, PCI_BASE_ADDRESS_0);
 if (!iobase)
 return;
@@ -183,8 +185,6 @@ init_lsi_scsi(struct pci_device *pci)
 int i;
 for (i = 0; i < 7; i++)
 lsi_scsi_scan_target(pci, iobase, i);
-
-return;
 }
 
 void
@@ -201,6 +201,6 @@ lsi_scsi_setup(void)
 if (pci->vendor != PCI_VENDOR_ID_LSI_LOGIC
 || pci->device != PCI_DEVICE_ID_LSI_53C895A)
 continue;
-init_lsi_scsi(pci);
+run_thread(init_lsi_scsi, pci);
 }
 }
diff --git a/src/hw/megasas.c b/src/hw/megasas.c
index 7514164..efd0f6e 100644
--- a/src/hw/megasas.c
+++ b/src/hw/megasas.c
@@ -357,8 +357,9 @@ static int megasas_transition_to_ready(struct pci_device 
*pci, u32 ioaddr)
 }
 
 static void
-init_megasas(struct pci_device *pci)
+init_megasas(void *data)
 {
+struct pci_device *pci = data;
 u32 bar = PCI_BASE_ADDRESS_2;
 if (!(pci_config_readl(pci->bdf, bar) & PCI_BASE_ADDRESS_IO_MASK))
 bar = PCI_BASE_ADDRESS_0;
@@ -372,8 +373,6 @@ init_megasas(struct pci_device *pci)
 // reset
 if (megasas_transition_to_ready(pci, iobase) == 0)
 megasas_scan_target(pci, iobase);
-
-return;
 }
 
 void
@@ -401,6 +400,6 @@ megasas_setup(void)
 pci->device == PCI_DEVICE_ID_DELL_PERC5 ||
 pci->device == PCI_DEVICE_ID_LSI_SAS2208 ||
 pci->device == PCI_DEVICE_ID_LSI_SAS3108)
-init_megasas(pci);
+run_thread(init_megasas, pci);
 }
 }
diff --git a/src/hw/mpt-scsi.c b/src/hw/mpt-scsi.c
index 7d1aa3d..a37e44c 100644
--- a/src/hw/mpt-scsi.c
+++ b/src/hw/mpt-scsi.c
@@ -15,6 +15,7 @@
 #include "pcidevice.h" // foreachpci
 #include "pci_ids.h" // PCI_DEVICE_ID
 #include "pci_regs.h" // PCI_VENDOR_ID
+#include "stacks.h" // run_thread
 #include "std/disk.h" // DISK_RET_SUCCESS
 #include "string.h" // memset
 #include "util.h" // usleep
@@ -241,8 +242,9 @@ mpt_out_doorbell(u8 func, u8 arg, u16 iobase)
 }
 
 static void
-init_mpt_scsi(struct pci_device *pci, const char *dev_name)
+init_mpt_scsi(void *data)
 {
+struct pci_device *pci = data;
 u16 *msg_in_p;
 u32 iobase = pci_enable_iobar(pci, PCI_BASE_ADDRESS_0);
 if (!iobase)
@@ -250,7 +252,8 @@ init_mpt_scsi(struct pci_device *pci, const char *dev_name)
 struct MptIOCInitReply MptIOCInitReply;
 pci_enable_busmaster(pci);
 
-dprintf(1, "found %s at %pP, io @ %x\n", dev_name, pci, iobase);
+dprintf(1, "found mpt-scsi(%04x) at %pP, io @ %x\n"
+, pci->device, pci, iobase);
 
 // reset
 mpt_out_doorbell(MPT_DOORBELL_MSG_RESET, 0, iobase);
@@ -295,16 +298,10 @@ mpt_scsi_setup(void)
 
 struct pci_dev

[SeaBIOS] [PATCH 2/2] shadow: Batch PCI config writes

[Kevin O'Connor]

Enabling and disabling shadow ram on QEMU is slow.  Batch the PCI
writes to reduce the number of memory changes QEMU must implement.

Signed-off-by: Kevin O'Connor 
---
 src/fw/shadow.c | 55 ++-
 1 file changed, 38 insertions(+), 17 deletions(-)

diff --git a/src/fw/shadow.c b/src/fw/shadow.c
index f766bb5..cd02d3a 100644
--- a/src/fw/shadow.c
+++ b/src/fw/shadow.c
@@ -21,27 +21,39 @@
 // On the emulators, the bios at 0xf is also at 0x
 #define BIOS_SRC_OFFSET 0xfff0
 
+union pamdata_u {
+u8 data8[8];
+u32 data32[2];
+};
+
 // Enable shadowing and copy bios.
 static void
 __make_bios_writable_intel(u16 bdf, u32 pam0)
 {
+// Read in current PAM settings from pci config space
+union pamdata_u pamdata;
+pamdata.data32[0] = pci_config_readl(bdf, ALIGN_DOWN(pam0, 4));
+pamdata.data32[1] = pci_config_readl(bdf, ALIGN_DOWN(pam0, 4) + 4);
+u8 *pam = &pamdata.data8[pam0 & 0x03];
+
 // Make ram from 0xc-0xf writable
 int i;
-for (i=0; i<6; i++) {
-u32 pam = pam0 + 1 + i;
-pci_config_writeb(bdf, pam, 0x33);
-}
+for (i=0; i<6; i++)
+pam[i + 1] = 0x33;
 
 // Make ram from 0xf-0x10 writable
-int reg = pci_config_readb(bdf, pam0);
-pci_config_writeb(bdf, pam0, 0x30);
-if (reg & 0x10)
-// Ram already present.
-return;
-
-// Copy bios.
-memcpy(VSYMBOL(code32flat_start), VSYMBOL(code32flat_start) + 
BIOS_SRC_OFFSET
-   , SYMBOL(code32flat_end) - SYMBOL(code32flat_start));
+int ram_present = pam[0] & 0x10;
+pam[0] = 0x30;
+
+// Write PAM settings back to pci config space
+pci_config_writel(bdf, ALIGN_DOWN(pam0, 4), pamdata.data32[0]);
+pci_config_writel(bdf, ALIGN_DOWN(pam0, 4) + 4, pamdata.data32[1]);
+
+if (!ram_present)
+// Copy bios.
+memcpy(VSYMBOL(code32flat_start)
+   , VSYMBOL(code32flat_start) + BIOS_SRC_OFFSET
+   , SYMBOL(code32flat_end) - SYMBOL(code32flat_start));
 }
 
 static void
@@ -68,6 +80,12 @@ make_bios_readonly_intel(u16 bdf, u32 pam0)
 // Flush any pending writes before locking memory.
 wbinvd();
 
+// Read in current PAM settings from pci config space
+union pamdata_u pamdata;
+pamdata.data32[0] = pci_config_readl(bdf, ALIGN_DOWN(pam0, 4));
+pamdata.data32[1] = pci_config_readl(bdf, ALIGN_DOWN(pam0, 4) + 4);
+u8 *pam = &pamdata.data8[pam0 & 0x03];
+
 // Write protect roms from 0xc-0xf
 u32 romlast = BUILD_BIOS_ADDR, rommax = BUILD_BIOS_ADDR;
 if (CONFIG_WRITABLE_UPPERMEMORY)
@@ -77,17 +95,20 @@ make_bios_readonly_intel(u16 bdf, u32 pam0)
 int i;
 for (i=0; i<6; i++) {
 u32 mem = BUILD_ROM_START + i * 32*1024;
-u32 pam = pam0 + 1 + i;
 if (romlast < mem + 16*1024 || rommax < mem + 32*1024) {
 if (romlast >= mem && rommax >= mem + 16*1024)
-pci_config_writeb(bdf, pam, 0x31);
+pam[i + 1] = 0x31;
 break;
 }
-pci_config_writeb(bdf, pam, 0x11);
+pam[i + 1] = 0x11;
 }
 
 // Write protect 0xf-0x10
-pci_config_writeb(bdf, pam0, 0x10);
+pam[0] = 0x10;
+
+// Write PAM settings back to pci config space
+pci_config_writel(bdf, ALIGN_DOWN(pam0, 4), pamdata.data32[0]);
+pci_config_writel(bdf, ALIGN_DOWN(pam0, 4) + 4, pamdata.data32[1]);
 }
 
 static int ShadowBDF = -1;
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 1/2] optionroms: Drop support for CONFIG_OPTIONROMS_DEPLOYED

[Kevin O'Connor]

No modern software uses this option and it complicates the code.

Signed-off-by: Kevin O'Connor 
---
 src/Kconfig  |  8 
 src/fw/shadow.c  | 15 +-
 src/optionroms.c | 61 
 3 files changed, 23 insertions(+), 61 deletions(-)

diff --git a/src/Kconfig b/src/Kconfig
index de39b38..e767be1 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -395,14 +395,6 @@ menu "BIOS interfaces"
 default y
 help
 Support finding and running option roms during POST.
-config OPTIONROMS_DEPLOYED
-depends on OPTIONROMS && QEMU
-bool "Option roms are already at 0xc-0xf"
-default n
-help
-Select this if option ROMs are already copied to
-0xc-0xf.  This must only be selected when using
-Bochs or QEMU versions older than 0.12.
 config PMM
 depends on OPTIONROMS
 bool "PMM interface"
diff --git a/src/fw/shadow.c b/src/fw/shadow.c
index bdb5c5b..f766bb5 100644
--- a/src/fw/shadow.c
+++ b/src/fw/shadow.c
@@ -26,24 +26,11 @@ static void
 __make_bios_writable_intel(u16 bdf, u32 pam0)
 {
 // Make ram from 0xc-0xf writable
-int clear = 0;
 int i;
 for (i=0; i<6; i++) {
 u32 pam = pam0 + 1 + i;
-int reg = pci_config_readb(bdf, pam);
-if (CONFIG_OPTIONROMS_DEPLOYED && (reg & 0x11) != 0x11) {
-// Need to copy optionroms to work around qemu implementation
-void *mem = (void*)(BUILD_ROM_START + i * 32*1024);
-memcpy((void*)BUILD_BIOS_TMP_ADDR, mem, 32*1024);
-pci_config_writeb(bdf, pam, 0x33);
-memcpy(mem, (void*)BUILD_BIOS_TMP_ADDR, 32*1024);
-clear = 1;
-} else {
-pci_config_writeb(bdf, pam, 0x33);
-}
+pci_config_writeb(bdf, pam, 0x33);
 }
-if (clear)
-memset((void*)BUILD_BIOS_TMP_ADDR, 0, 32*1024);
 
 // Make ram from 0xf-0x10 writable
 int reg = pci_config_readb(bdf, pam0);
diff --git a/src/optionroms.c b/src/optionroms.c
index 9897753..65f7fe0 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -347,28 +347,16 @@ optionrom_setup(void)
 memset(sources, 0, sizeof(sources));
 u32 post_vga = rom_get_last();
 
-if (CONFIG_OPTIONROMS_DEPLOYED) {
-// Option roms are already deployed on the system.
-u32 pos = post_vga;
-while (pos < rom_get_max()) {
-int ret = init_optionrom((void*)pos, 0, 0);
-if (ret)
-pos += OPTION_ROM_ALIGN;
-else
-pos = rom_get_last();
-}
-} else {
-// Find and deploy PCI roms.
-struct pci_device *pci;
-foreachpci(pci) {
-if (pci->class == PCI_CLASS_DISPLAY_VGA || pci->have_driver)
-continue;
-init_pcirom(pci, 0, sources);
-}
-
-// Find and deploy CBFS roms not associated with a device.
-run_file_roms("genroms/", 0, sources);
+// Find and deploy PCI roms.
+struct pci_device *pci;
+foreachpci(pci) {
+if (pci->class == PCI_CLASS_DISPLAY_VGA || pci->have_driver)
+continue;
+init_pcirom(pci, 0, sources);
 }
+
+// Find and deploy CBFS roms not associated with a device.
+run_file_roms("genroms/", 0, sources);
 rom_reserve(0);
 
 // All option roms found and deployed - now build BEV/BCV vectors.
@@ -427,26 +415,21 @@ vgarom_setup(void)
 RunPCIroms = romfile_loadint("etc/pci-optionrom-exec", 2);
 ScreenAndDebug = romfile_loadint("etc/screen-and-debug", 1);
 
-if (CONFIG_OPTIONROMS_DEPLOYED) {
-// Option roms are already deployed on the system.
-init_optionrom((void*)BUILD_ROM_START, 0, 1);
-} else {
-// Clear option rom memory
-memset((void*)BUILD_ROM_START, 0, rom_get_max() - BUILD_ROM_START);
-
-// Find and deploy PCI VGA rom.
-struct pci_device *pci;
-foreachpci(pci) {
-if (!is_pci_vga(pci))
-continue;
-vgahook_setup(pci);
-init_pcirom(pci, 1, NULL);
-break;
-}
+// Clear option rom memory
+memset((void*)BUILD_ROM_START, 0, rom_get_max() - BUILD_ROM_START);
 
-// Find and deploy CBFS vga-style roms not associated with a device.
-run_file_roms("vgaroms/", 1, NULL);
+// Find and deploy PCI VGA rom.
+struct pci_device *pci;
+foreachpci(pci) {
+if (!is_pci_vga(pci))
+continue;
+vgahook_setup(pci);
+init_pcirom(pci, 1, NULL);
+break;
 }
+
+// Find and deploy CBFS vga-style roms not associated with a device.
+run_file_roms("vgaroms/", 1, NULL);
 rom_reserve(0);
 
 if (rom_get_last() == BUILD_ROM_START)
-- 
2.5.5

Re: [SeaBIOS] virtio-blk ... block size 4096 is unsupported

[Kevin O'Connor]

On Tue, Apr 05, 2016 at 02:31:53PM +0200, Laszlo Ersek wrote:
> On 04/05/16 06:25, James Shimer wrote:
> > I've been doing some testing if KVM with 4K physical sector virtio
> > disks and they work with KVM as a none boot disk.  We'd like to be
> > able to boot a 4k physical sector disk in KVM, is there 4K support in
> > any seabios newer/upcoming releases, or is there a way to configure
> > for 4k sectors?
> > 
> > reference from master
> > src/hw/virtio-blk.c lin 142
> > 
> >if (vdrive->drive.blksize != DISK_SECTOR_SIZE) {
> > dprintf(1, "virtio-blk %pP block size %d is unsupported\n",
> > pci, vdrive->drive.blksize);
> > goto fail;
> > }
> 
> This code dates back to 4030db0d2c5a7.
> 
> I'm not an expert, but I think BIOS interfaces don't support a sector
> size different from 512 B. It seems that a drive with 4096 B physical
> sectors should enable (the emulation of) a 512 B logical sector size, so
> that it can work with code that assumes an 512 B sector size.
> 
>   https://en.wikipedia.org/wiki/Advanced_Format#512e

The EDD standard ( https://en.wikipedia.org/wiki/Enhanced_Disk_Drive )
allows for 4K blocks.  However, the older BIOS interface (eg, int
1301) only support 512 byte sectors.  This means that the BIOS would
need to emulate 512 byte reads should one of the older interfaces be
called on one of these drives.  This isn't difficult as SeaBIOS
already does this for cdroms.

> I guess for a virtio-blk disk, that means:
> 
>   -device virtio-blk-pci,physical_block_size=4096,logical_block_size=512

In practice, almost all real drives seem to do the above.  So, adding
support for drives that don't has not previously been a priority.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 2/2] disk: Avoid stack_hop() path if already on the extra stack

[Kevin O'Connor]

If CONFIG_ENTRY_EXTRASTACK is set (enabled by default) then the 16bit
disk interface code is already running on the extra stack and it is
not necessary to support stack switching on each disk request.

Signed-off-by: Kevin O'Connor 
---
 src/block.c |  4 
 src/disk.c  | 12 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/block.c b/src/block.c
index a383574..f7280cf 100644
--- a/src/block.c
+++ b/src/block.c
@@ -599,6 +599,10 @@ process_op_16(struct disk_op_s *op)
 int
 process_op(struct disk_op_s *op)
 {
+dprintf(DEBUG_HDL_13, "disk_op d=%p lba=%d buf=%p count=%d cmd=%d\n"
+, op->drive_gf, (u32)op->lba, op->buf_fl
+, op->count, op->command);
+
 int ret, origcount = op->count;
 if (origcount * GET_GLOBALFLAT(op->drive_gf->blksize) > 64*1024) {
 op->count = 0;
diff --git a/src/disk.c b/src/disk.c
index bcd6a09..dc0427c 100644
--- a/src/disk.c
+++ b/src/disk.c
@@ -87,18 +87,12 @@ getLCHS(struct drive_s *drive_gf)
 return res;
 }
 
-// Execute a "disk_op_s" request - this runs on the extra stack.
+// Execute a "disk_op_s" request after jumping to the extra stack.
 static int
 __send_disk_op(struct disk_op_s *op_far, u16 op_seg)
 {
 struct disk_op_s dop;
-memcpy_far(GET_SEG(SS), &dop
-   , op_seg, op_far
-   , sizeof(dop));
-
-dprintf(DEBUG_HDL_13, "disk_op d=%p lba=%d buf=%p count=%d cmd=%d\n"
-, dop.drive_gf, (u32)dop.lba, dop.buf_fl
-, dop.count, dop.command);
+memcpy_far(GET_SEG(SS), &dop, op_seg, op_far, sizeof(dop));
 
 int status = process_op(&dop);
 
@@ -115,6 +109,8 @@ send_disk_op(struct disk_op_s *op)
 ASSERT16();
 if (! CONFIG_DRIVES)
 return -1;
+if (CONFIG_ENTRY_EXTRASTACK)
+return process_op(op);
 
 return stack_hop(__send_disk_op, op, GET_SEG(SS));
 }
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

[SeaBIOS] [PATCH 1/2] block: Move send_disk_op() from block.c to disk.c

[Kevin O'Connor]

The send_disk_op() function is only called from the 16bit handlers
found in disk.c.

Signed-off-by: Kevin O'Connor 
---
 src/block.c | 34 +-
 src/block.h |  1 -
 src/disk.c  | 32 
 3 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/src/block.c b/src/block.c
index d9de29f..a383574 100644
--- a/src/block.c
+++ b/src/block.c
@@ -22,7 +22,7 @@
 #include "hw/virtio-scsi.h" // virtio_scsi_process_op
 #include "malloc.h" // malloc_low
 #include "output.h" // dprintf
-#include "stacks.h" // stack_hop
+#include "stacks.h" // call32
 #include "std/disk.h" // struct dpte_s
 #include "string.h" // checksum
 #include "util.h" // process_floppy_op
@@ -613,35 +613,3 @@ process_op(struct disk_op_s *op)
 op->count = 0;
 return ret;
 }
-
-// Execute a "disk_op_s" request - this runs on the extra stack.
-static int
-__send_disk_op(struct disk_op_s *op_far, u16 op_seg)
-{
-struct disk_op_s dop;
-memcpy_far(GET_SEG(SS), &dop
-   , op_seg, op_far
-   , sizeof(dop));
-
-dprintf(DEBUG_HDL_13, "disk_op d=%p lba=%d buf=%p count=%d cmd=%d\n"
-, dop.drive_gf, (u32)dop.lba, dop.buf_fl
-, dop.count, dop.command);
-
-int status = process_op(&dop);
-
-// Update count with total sectors transferred.
-SET_FARVAR(op_seg, op_far->count, dop.count);
-
-return status;
-}
-
-// Execute a "disk_op_s" request by jumping to the extra 16bit stack.
-int
-send_disk_op(struct disk_op_s *op)
-{
-ASSERT16();
-if (! CONFIG_DRIVES)
-return -1;
-
-return stack_hop(__send_disk_op, op, GET_SEG(SS));
-}
diff --git a/src/block.h b/src/block.h
index a5f38c4..0f15ff9 100644
--- a/src/block.h
+++ b/src/block.h
@@ -115,7 +115,6 @@ int fill_edd(struct segoff_s edd, struct drive_s *drive_gf);
 void block_setup(void);
 int default_process_op(struct disk_op_s *op);
 int process_op(struct disk_op_s *op);
-int send_disk_op(struct disk_op_s *op);
 int create_bounce_buf(void);
 
 #endif // block.h
diff --git a/src/disk.c b/src/disk.c
index 3854d00..bcd6a09 100644
--- a/src/disk.c
+++ b/src/disk.c
@@ -87,6 +87,38 @@ getLCHS(struct drive_s *drive_gf)
 return res;
 }
 
+// Execute a "disk_op_s" request - this runs on the extra stack.
+static int
+__send_disk_op(struct disk_op_s *op_far, u16 op_seg)
+{
+struct disk_op_s dop;
+memcpy_far(GET_SEG(SS), &dop
+   , op_seg, op_far
+   , sizeof(dop));
+
+dprintf(DEBUG_HDL_13, "disk_op d=%p lba=%d buf=%p count=%d cmd=%d\n"
+, dop.drive_gf, (u32)dop.lba, dop.buf_fl
+, dop.count, dop.command);
+
+int status = process_op(&dop);
+
+// Update count with total sectors transferred.
+SET_FARVAR(op_seg, op_far->count, dop.count);
+
+return status;
+}
+
+// Execute a "disk_op_s" request by jumping to the extra 16bit stack.
+static int
+send_disk_op(struct disk_op_s *op)
+{
+ASSERT16();
+if (! CONFIG_DRIVES)
+return -1;
+
+return stack_hop(__send_disk_op, op, GET_SEG(SS));
+}
+
 // Perform read/write/verify using old-style chs accesses
 static void noinline
 basic_access(struct bregs *regs, struct drive_s *drive_gf, u16 command)
-- 
2.5.5


___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] Build failed in Jenkins: seabios #220

[Kevin O'Connor]

On Wed, Mar 30, 2016 at 07:50:33PM +0200, Jenkins Build Host wrote:
> See 
> 
> Changes:
> 
> [kevin] Support for booting from LSI Logic LSI53C1030, SAS1068, SAS1068e
[...]
>   Compile checking out/src/hw/mpt-scsi.o
> src/hw/mpt-scsi.c: In function 'init_mpt_scsi':
> src/hw/mpt-scsi.c:281:5: error: 'for' loop initial declarations are only 
> allowed in C99 mode
>  for (int i = 0; i < 7; i++)
>  ^
> src/hw/mpt-scsi.c:281:5: note: use option -std=c99 or -std=gnu99 to compile 
> your code
> make: *** [out/src/hw/mpt-scsi.o] Error 1
> error: Bad exit status from /var/tmp/rpm-tmp.YLsXMX (%build)

It's interesting that newer compilers accept the 'int i' declaration
inside of for loops - I didn't realize that was allowed in C99.  In
any case, I pulled the declaration outside the loop to suppress the
error, and I committed the change.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] Support for booting from LSI Logic LSI53C1030, SAS1068, SAS1068e

[Kevin O'Connor]

On Tue, Mar 29, 2016 at 03:11:06PM +0200, Paolo Bonzini wrote:
> On 29/03/2016 14:43, Kevin O'Connor wrote:
> >> > Also known as Fusion MPT disk; this controller model is supported
> >> > by VirtualBox and VMware, and QEMU support patches have been
> >> > posted.
> > Thanks Paolo.  The patch didn't apply - I took a stab at updating it
> > and pushed the patch to:
> > 
> > https://github.com/KevinOConnor/seabios/tree/testing
> > 
> > I also added your copyright to the new file and made the code
> > dependent on QEMU_HARDWARE.
> > 
> > Can you verify the code still works and confirm if you are okay with
> > these changes?
> 
> Yes, it works.  Regarding the copyright notice, please add
> 
>  * Copyright (c) 2012 Verizon, Inc.
> 
> as well.  Thanks very much, I didn't know about the new PCI helpers!

Thanks.  I committed this patch.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] Support for booting from LSI Logic LSI53C1030, SAS1068, SAS1068e

[Kevin O'Connor]

On Fri, Mar 25, 2016 at 05:04:31PM +0100, Paolo Bonzini wrote:
> From: Don Slutz 
> 
> Also known as Fusion MPT disk; this controller model is supported
> by VirtualBox and VMware, and QEMU support patches have been
> posted.

Thanks Paolo.  The patch didn't apply - I took a stab at updating it
and pushed the patch to:

https://github.com/KevinOConnor/seabios/tree/testing

I also added your copyright to the new file and made the code
dependent on QEMU_HARDWARE.

Can you verify the code still works and confirm if you are okay with
these changes?

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] sdcard: abort controller setup if capabilities invalid

[Kevin O'Connor]

On Wed, Mar 16, 2016 at 08:53:09PM -0500, Matt DeVillier wrote:
> If the version and low/high capabilities flags of a sdcard controller are
> invalid (0x), assume the controller address is invalid, and exit setup
> before attempting to reset the controller, which would introduce an
> unnecessary
> delay, since the reset would ultimately fail after timing out.
> 
> This eliminates the delay in displaying the boot menu message on Baytrail
> ChromeOS devices, where multiple /etc/sdcard entries are present in a single
> payload to cover the range of eMMC controller addresses used.
> 
> Signed-off-by: Matt DeVillier 

I think this is a good idea, but I don't think we can rely on the
memory having all 0xff.  At a minimum I think 0x00 should be checked
for as well, but ideally the controller would have a signature we
could check.

BTW, the patch got white-space mangled.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [patch v2 - reformatted] sdcard: skip detection of PCI sdhci controllers if etc/sdcard used

[Kevin O'Connor]

On Wed, Mar 16, 2016 at 08:37:35PM -0500, Matt DeVillier wrote:
> Some BayTrail ChromeOS devices have the eMMC controller hidden (thus
> requiring the use of etc/sdcard), while others do not, making it problematic
> to have a single payload which serves all devices properly.   Therefore, if
> the CBFS contains etc/sdcard entries, skip detection of any visible PCI
> sdhci controllers in order to avoid duplicate entries in the boot menu.
> 
> patch implementation suggested by Kevin O'Connor :)
> 
> Signed-off-by: Matt DeVillier 

Thanks.  This patch got whitespace mangled as well.  However, I
applied the patch after modifying it slightly and updating the
documentation.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] coreboot: Add support for FMAP and multiple CBFS

[Kevin O'Connor]

On Thu, Mar 10, 2016 at 11:11:59AM -0600, Ben Gardner wrote:
> Hi Kevin,
> 
> I plan to have the default CBFS contain only coreboot and SeabIOS and
> a few payloads (coreinfo, memtest86, PXE) that I don't intend to
> change often.
> That would be protected with a hash and would be kept small for
> performance reasons.
> ChromeOS appears to do something similar and take it a step further
> and write-protect that "read only" region of flash.
> 
> The other CBFS would contain larger images that I'm likely to change,
> such as Linux.
> 
> There is a chance of name conflicts and that may cause problems.
> In my use case, the answer is "don't do that".
> 
> From a random sampling of FMD files in the coreboot tree
> (google/glados/chromeos.fmd), I see that ChromeOS uses three:
> FW_MAIN_A, FW_MAIN_B, and the default COREBOOT.
> Assuming FW_MAIN_A and FW_MAIN_B contain files with similar names,
> that would cause confusion if they use SeaBIOS.
> But I think that ChromeOS is switching to use the Depthcharge bootloader.
> 
> So, maybe this wouldn't work well for everyone.

Thanks.  If this may not work for everyone, then I suggest adding a
build time (kconfig) or runtime (cbfs) setting to enable/disable it.
Normally runtime settings are preferred, but that may not work here.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] coreboot: Add support for FMAP and multiple CBFS

[Kevin O'Connor]

On Wed, Mar 09, 2016 at 12:19:43PM -0600, Ben Gardner wrote:
> ROM images with a FMAP may have multiple CBFS.
> Scan all available CBFS so that, say, a SeaBIOS bootable image doesn't
> have to be in the main CBFS.
> 
> Coreboot puts the FMAP location in the BOOT_MEDIA_PARAMS entry in the
> coreboot table. We can grab that and can all regions for CBFS files.

Thanks.  Can you provide some additional background information on how
this is intended to work.  If multiple CBFS locations are found, is
there a chance of file name conflicts?  Is the fmap stuff used for
primary/backup rom images and if so how will seabios account for that
if it scans both images?

Also, can you describe the high level use case you had in mind.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [patch] sdcard: skip detection of PCI sdhci controllers if etc/sdcard used

[Kevin O'Connor]

On Thu, Mar 10, 2016 at 12:38:14AM -0600, Matt DeVillier wrote:
> Some BayTrail ChromeOS devices have the eMMC controller hidden (thus
> requiring the use of etc/sdcard), while others do not, making it problematic
> to have a single payload which serves all devices properly.  Therefore, if
> the CBFS contains etc/sdcard entries, skip detection of any visible PCI
> sdhci controllers in order to avoid duplicate entries in the boot menu.

John, would disabling PCI scans for sd controllers when any etc/sdcard
files exist work okay for the machines you've tested with?

[...]
> diff --git a/src/hw/sdcard.c b/src/hw/sdcard.c
> index 7e0875f..32cdb14 100644
> --- a/src/hw/sdcard.c
> +++ b/src/hw/sdcard.c
> @@ -557,11 +557,14 @@ sdcard_setup(void)
>  run_thread(sdcard_romfile_setup, file);
>  }
> 
> -struct pci_device *pci;
> -foreachpci(pci) {
> -if (pci->class != PCI_CLASS_SYSTEM_SDHCI || pci->prog_if >= 2)
> -// Not an SDHCI controller following SDHCI spec
> -continue;
> -run_thread(sdcard_pci_setup, pci);
> +//only scan for PCI controllers if etc/sdcard* not used
> +if (file == NULL) {

Unless I've missed something, this test (file==NULL) is always true
here.  Maybe add a counter or flag in the romfile search loop.

-Kevin

> +struct pci_device *pci;
> +foreachpci(pci) {
> +if (pci->class != PCI_CLASS_SYSTEM_SDHCI || pci->prog_if >= 2)
> +// Not an SDHCI controller following SDHCI spec
> +continue;
> +run_thread(sdcard_pci_setup, pci);
> +}
>  }
>  }

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH V2] fw/pci: add Q35 S3 support

[Kevin O'Connor]

On Tue, Mar 01, 2016 at 04:06:45PM +0200, Marcel Apfelbaum wrote:
> Following the i440fx example, save the LPC, SMBUS and PCIEXBAR bdfs
> between OS sleeps and use them to re-configure the
> corresponding registers.

Thanks.  I committed this patch.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] ahci: set transfer mode according to the capabilities of connected drive

[Kevin O'Connor]

On Sat, Feb 20, 2016 at 03:20:15PM +0100, Gerd Hoffmann wrote:
> Use case: cf cards behind sata-ide bridge, which might not support
> the default transfer mode.
> 
> Based on a patch by Werner Zeh ,
> with some minor tweaks applied.
> 
> Signed-off-by: Gerd Hoffmann 

Thanks.  I modified the indentation and committed this patch.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 2/2] tpm: Write logs in TPM 2 format

[Kevin O'Connor]

On Fri, Feb 26, 2016 at 01:28:14PM -0500, Stefan Berger wrote:
> On 02/23/2016 11:53 AM, Kevin O'Connor wrote:
> >commit 102648838c0cf640c01b5f092e8f432d8f2abb8f
> >Author: Kevin O'Connor 
> >Date:   Fri Feb 5 22:28:17 2016 -0500
> >
> > tpm: Write logs in TPM 2 format
> > Add support for the TPM 2 format of log messages.
> > Write the logs in the format that is appropriate for the version of
> > the host's TPM. For TPM 1.2 write it in the 'pcpes' structure's
> > format, for TPM 2 in the new TPM 2 format.
> > By using this method we can keep the API interface on systems with a
> > TPM 2 even though applications pass in the 'pcpes' structures
> > directly. The log will still be written in the appropriate format.
> > The TPM 2 log contains a TPM 1.2 type of entry of event type
> > EV_NO_ACTION and entry of type TCG_EfiSpeIdEventStruct as the first
> > entry. This is described in the EFI specification (section 5.3):
> > Signed-off-by: Kevin O'Connor 
> Signed-off-by: Stefan Berger 

Thanks.  I committed this patch.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] Serial keyboard support?

[Kevin O'Connor]

On Thu, Feb 25, 2016 at 10:02:34AM -0800, Dustin Brazeau wrote:
> As far as I can tell SeaBIOS does not currently support keyboard input over
> serial. I was wondering if there is an unofficial patch that could add
> support for this before I start looking into implementing it myself. Thanks.

For traditional serial ports, sgabios generally fills that role.  See
the SeaBIOS coreboot wiki for info on sgabios:

http://www.coreboot.org/SeaBIOS#Adding_sgabios_support

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] ld: fix .text section address alignment

[Kevin O'Connor]

On Tue, Feb 23, 2016 at 04:06:20PM +0100, Roger Pau Monné wrote:
> El 23/2/16 a les 15:53, Kevin O'Connor ha escrit:
> > On Mon, Feb 22, 2016 at 12:07:00PM +0100, Roger Pau Monné wrote:
> >> El 20/2/16 a les 3:41, Kevin O'Connor ha escrit:
> >>> On Tue, Feb 16, 2016 at 06:21:10PM +0100, Roger Pau Monné wrote:
> >>>> It seems like ELF toolchain objcopy chokes if a section address is not
> >>>> aligned to the alignment specified by the section, see:
> >>>>
> >>>> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=207170
> >>>>
> >>>> The snippet shown above has addr aligned to 16 (which matches latest
> >>>> upstream), so it's not a problem, but the current SeaBIOS version
> >>>> shipped in Xen 4.5 (1.7.5 IIRC) ends up with an addr that's not a
> >>>> multiple of 16, as shown in the bug report, and objcopy complains with:
> >>>>
> >>>> objcopy: elf_update() failed: Layout constraint violation
> >>>
> >>> Thanks.  I agree it should be fixed.  However, I think there are a few
> >>> other cases that could cause the ".text" section alignment to be off.
> >>> Are you okay with the patch below instead?
> >>
> >> Yes, looks fine to me. AFAICT SeaBIOS packs all the sections (.text,
> >> .data, .rodata) ibnside of the .text section, which I didn't realize 
> >> before.
> > 
> > Thanks, I committed the change.
> 
> Thanks, I would also like to request this fix to be backported to stable
> branches. Should I send a formal request, or is this email enough?
> 
> Ideally I would like to see it applied to 1.9, 1.8 and 1.7.5.

Gerd maintains the stable trees.

I don't know Gerd's thoughts on supporting older stable branches, but
I can say that the build changed significantly after 1.7.5 and it
would require more than a simple backport of this patch.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 2/2] tpm: Write logs in TPM 2 format

[Kevin O'Connor]

On Mon, Feb 08, 2016 at 11:36:40AM -0500, Kevin O'Connor wrote:
> On Mon, Feb 08, 2016 at 07:25:35AM -0500, Stefan Berger wrote:
> > On 02/06/2016 01:35 PM, Kevin O'Connor wrote:
> > >Add support for the TPM 2 format of log messages.
> > >
> > >Write the logs in the format that is appropriate for the version of
> > >the host's TPM. For TPM 1.2 write it in the 'pcpes' structure's
> > >format, for TPM 2 in the new TPM 2 format.
> > >
> > >By using this method we can keep the API interface on systems with a
> > >TPM 2 even though applications pass in the 'pcpes' structures
> > >directly. The log will still be written in the appropriate format.
> > >
> > >The TPM 2 log contains a TPM 1.2 type of entry of event type
> > >EV_NO_ACTION and entry of type TCG_EfiSpeIdEventStruct as the first
> > >entry. This is described in the EFI specification (section 5.3):
> > >
> > >Signed-off-by: Kevin O'Connor 

Hi Stefan,

I updated the patch with your feedback on an explicit version check
instead of using EV_NO_ACTION.

Are you okay with the patch below, and if so can I add your
signed-off-by line?

-Kevin


commit 102648838c0cf640c01b5f092e8f432d8f2abb8f
Author: Kevin O'Connor 
Date:   Fri Feb 5 22:28:17 2016 -0500

tpm: Write logs in TPM 2 format

Add support for the TPM 2 format of log messages.

Write the logs in the format that is appropriate for the version of
the host's TPM. For TPM 1.2 write it in the 'pcpes' structure's
format, for TPM 2 in the new TPM 2 format.

By using this method we can keep the API interface on systems with a
TPM 2 even though applications pass in the 'pcpes' structures
directly. The log will still be written in the appropriate format.

The TPM 2 log contains a TPM 1.2 type of entry of event type
EV_NO_ACTION and entry of type TCG_EfiSpeIdEventStruct as the first
entry. This is described in the EFI specification (section 5.3):

Signed-off-by: Kevin O'Connor 

diff --git a/src/std/tcg.h b/src/std/tcg.h
index dbb3a60..c59f671 100644
--- a/src/std/tcg.h
+++ b/src/std/tcg.h
@@ -91,6 +91,7 @@ enum irq_ids {
 
 /* event types: 10.4.1 / table 11 */
 #define EV_POST_CODE 1
+#define EV_NO_ACTION 3
 #define EV_SEPARATOR 4
 #define EV_ACTION5
 #define EV_EVENT_TAG 6
@@ -474,4 +475,38 @@ struct tpm2_req_hierarchycontrol {
 u8 state;
 } PACKED;
 
+/* TPM 2 log entry */
+
+struct tpml_digest_values_sha1 {
+u16 hashtype;
+u8 sha1[SHA1_BUFSIZE];
+};
+
+struct tcg_pcr_event2_sha1 {
+u32 pcrindex;
+u32 eventtype;
+u32 count; /* number of digests */
+struct tpml_digest_values_sha1 digests[1];
+u32 eventdatasize;
+u8 event[0];
+} PACKED;
+
+struct TCG_EfiSpecIdEventStruct {
+u8 signature[16];
+u32 platformClass;
+u8 specVersionMinor;
+u8 specVersionMajor;
+u8 specErrata;
+u8 uintnSize;
+u32 numberOfAlgorithms;
+struct TCG_EfiSpecIdEventAlgorithmSize {
+u16 algorithmId;
+u16 digestSize;
+} digestSizes[1];
+u8 vendorInfoSize;
+u8 vendorInfo[0];
+};
+
+#define TPM_TCPA_ACPI_CLASS_CLIENT 0
+
 #endif // tcg.h
diff --git a/src/tcgbios.c b/src/tcgbios.c
index cddc99b..ddf4f79 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -141,7 +141,8 @@ tpm_tcpa_probe(void)
  *  Returns an error code in case of faiure, 0 in case of success
  */
 static int
-tpm_log_event(struct pcpes *pcpes, const void *event)
+tpm_log_event(struct tcg_pcr_event2_sha1 *entry, const void *event
+  , TPMVersion tpm_version)
 {
 dprintf(DEBUG_tcg, "TCGBIOS: LASA = %p, next entry = %p\n",
 tpm_state.log_area_start_address, tpm_state.log_area_next_entry);
@@ -149,17 +150,30 @@ tpm_log_event(struct pcpes *pcpes, const void *event)
 if (tpm_state.log_area_next_entry == NULL)
 return -1;
 
-u32 size = sizeof(*pcpes) + pcpes->eventdatasize;
-
-if ((tpm_state.log_area_next_entry + size - 
tpm_state.log_area_start_address) >
- tpm_state.log_area_minimum_length) {
+u32 size = sizeof(*entry) + entry->eventdatasize;
+u32 logsize = (tpm_state.log_area_next_entry + size
+   - tpm_state.log_area_start_address);
+if (logsize > tpm_state.log_area_minimum_length) {
 dprintf(DEBUG_tcg, "TCGBIOS: LOG OVERFLOW: size = %d\n", size);
 return -1;
 }
 
-memcpy(tpm_state.log_area_next_entry, pcpes, sizeof(*pcpes));
-memcpy(tpm_state.log_area_next_entry + sizeof(*pcpes),
-   event, pcpes->eventdatasize);
+switch (tpm_version) {
+case TPM_VERSION_1_2: ;
+struct pcpes *pcpes = (void*)tpm_state.log_area_next_entry;
+pcpes->pcrindex = entry->p

Re: [SeaBIOS] [RFC PATCH v4] fw/pci: Add support for mapping Intel IGD via QEMU

[Kevin O'Connor]

On Tue, Feb 16, 2016 at 02:39:27PM -0700, Alex Williamson wrote:
> QEMU provides two fw_cfg files to support IGD.  The first holds the
> OpRegion data which holds the Video BIOS Table (VBT).  This needs to
> be copied into reserved memory and the address stored in the ASL
> Storage register of the device at 0xFC offset in PCI config space.
> The OpRegion is generally 8KB.  This file is named "etc/igd-opregion".
> 
> The second file tells us the required size of the stolen memory space
> for the device.  This is a dummy file, it has no backing so we only
> allocate the space without copying anything into it.  This space
> requires 1MB alignment and is generally either 1MB or 2MB, depending
> on the hardware config.  If the user has opted in QEMU to expose
> additional stolen memory beyond the GTT (GGMS), the GMS may add an
> additional 32MB to 512MB.  The base address of the reserved memory
> allocated for this is written back to the Base Data of Stolen Memory
> register (BDSM) at PCI config offset 0x5C on the device.  This file is
> named "etc/igd-bdsm".

Thanks.  I'd be interested in seeing how the discussion with Michael
and Igor works out (wrt using a standardized interface for
communicating addresses between qemu and firmware) before adopting
this interface in SeaBIOS.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] ld: fix .text section address alignment

[Kevin O'Connor]

On Mon, Feb 22, 2016 at 12:07:00PM +0100, Roger Pau Monné wrote:
> El 20/2/16 a les 3:41, Kevin O'Connor ha escrit:
> > On Tue, Feb 16, 2016 at 06:21:10PM +0100, Roger Pau Monné wrote:
> >> El 16/2/16 a les 17:33, Kevin O'Connor ha escrit:
> >>> On Tue, Feb 16, 2016 at 01:56:26PM +0100, Roger Pau Monne wrote:
> >>>> According to the output from readelf, the .text section should be 
> >>>> aligned to
> >>>> 16:
> >>>>
> >>>> Section Headers:
> >>>>   [Nr] Name  TypeAddr OffSize   ES Flg 
> >>>> Lk Inf Al
> >>>>   [ 0] (null)NULL 00 00 00  
> >>>> 0   0  0
> >>>>   [ 1] .text PROGBITS000de300 000300 021d00 00  AX  
> >>>> 0   0 16
> >>>> [...]
> >>>>
> >>>> This however doesn't seem to be enforced when the relocations are 
> >>>> generated.
> >>>> The following patch tries to address this by making sure the space used 
> >>>> for
> >>>> the relocations it also aligned to the same value as the .text section.
> >>>
> >>> Thanks.  What goes wrong if the .text section is not aligned?  The
> >>> code has already been assigned physical addresses by this point, so it
> >>> should not impact the runtime code.
> >>
> >> It seems like ELF toolchain objcopy chokes if a section address is not
> >> aligned to the alignment specified by the section, see:
> >>
> >> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=207170
> >>
> >> The snippet shown above has addr aligned to 16 (which matches latest
> >> upstream), so it's not a problem, but the current SeaBIOS version
> >> shipped in Xen 4.5 (1.7.5 IIRC) ends up with an addr that's not a
> >> multiple of 16, as shown in the bug report, and objcopy complains with:
> >>
> >> objcopy: elf_update() failed: Layout constraint violation
> > 
> > Thanks.  I agree it should be fixed.  However, I think there are a few
> > other cases that could cause the ".text" section alignment to be off.
> > Are you okay with the patch below instead?
> 
> Yes, looks fine to me. AFAICT SeaBIOS packs all the sections (.text,
> .data, .rodata) ibnside of the .text section, which I didn't realize before.

Thanks, I committed the change.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] ahci: set transfer mode according to the capabilities of connected drive

[Kevin O'Connor]

On Sat, Feb 20, 2016 at 03:20:15PM +0100, Gerd Hoffmann wrote:
> Use case: cf cards behind sata-ide bridge, which might not support
> the default transfer mode.
> 
> Based on a patch by Werner Zeh ,
> with some minor tweaks applied.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  src/hw/ahci.c | 58 ++
>  src/hw/ata.h  |  5 +
>  2 files changed, 63 insertions(+)
> 
> diff --git a/src/hw/ahci.c b/src/hw/ahci.c
> index 9310850..ea862f2 100644
> --- a/src/hw/ahci.c
> +++ b/src/hw/ahci.c
> @@ -515,6 +515,64 @@ static int ahci_port_setup(struct ahci_port_s *port)
>, ata_extract_version(buffer)
>, (u32)adjsize, adjprefix);
>  port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0);
> +
> +s8 multi_dma = -1;
> +s8 pio_mode = -1;
> +s8 udma_mode = -1;
> +// If bit 2 in word 53 is set, udma information is valid in word 88.
> +if (buffer[53] & 0x04) {
> +udma_mode = 6;
> +while ((udma_mode >= 0) &&
> +!((buffer[88] & 0x7f) & ( 1 << udma_mode ))) {
> +udma_mode--;
> +}
> +}
> +// If bit 1 in word 53 is set, multiword-dma and advanced pio modes
> +// are available in words 63 and 64.
> +if (buffer[53] & 0x02) {
> +pio_mode = 4;
> +multi_dma = 3;
> +while ((multi_dma >= 0) &&
> +   !((buffer[63] & 0x7) & ( 1 << multi_dma ))) {
> +multi_dma--;
> +}

This indentation should be fixed.

Otherwise, looks good to me.
-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] ld: fix .text section address alignment

[Kevin O'Connor]

On Tue, Feb 16, 2016 at 06:21:10PM +0100, Roger Pau Monné wrote:
> El 16/2/16 a les 17:33, Kevin O'Connor ha escrit:
> > On Tue, Feb 16, 2016 at 01:56:26PM +0100, Roger Pau Monne wrote:
> >> According to the output from readelf, the .text section should be aligned 
> >> to
> >> 16:
> >>
> >> Section Headers:
> >>   [Nr] Name  TypeAddr OffSize   ES Flg Lk 
> >> Inf Al
> >>   [ 0] (null)NULL 00 00 00  0  
> >>  0  0
> >>   [ 1] .text PROGBITS000de300 000300 021d00 00  AX  0  
> >>  0 16
> >> [...]
> >>
> >> This however doesn't seem to be enforced when the relocations are 
> >> generated.
> >> The following patch tries to address this by making sure the space used for
> >> the relocations it also aligned to the same value as the .text section.
> > 
> > Thanks.  What goes wrong if the .text section is not aligned?  The
> > code has already been assigned physical addresses by this point, so it
> > should not impact the runtime code.
> 
> It seems like ELF toolchain objcopy chokes if a section address is not
> aligned to the alignment specified by the section, see:
> 
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=207170
> 
> The snippet shown above has addr aligned to 16 (which matches latest
> upstream), so it's not a problem, but the current SeaBIOS version
> shipped in Xen 4.5 (1.7.5 IIRC) ends up with an addr that's not a
> multiple of 16, as shown in the bug report, and objcopy complains with:
> 
> objcopy: elf_update() failed: Layout constraint violation

Thanks.  I agree it should be fixed.  However, I think there are a few
other cases that could cause the ".text" section alignment to be off.
Are you okay with the patch below instead?

-Kevin


commit 3910de0dee216d5b5bf23cfa29bfc80d082b2ee7
Author: Kevin O'Connor 
Date:   Fri Feb 19 21:34:16 2016 -0500

build: fix .text section address alignment

Some linkers verify that sections have a start address that is aligned
with the minimum alignment of that section.  Add extra padding to the
".text" section to ensure it is always aligned with the maximum
alignment of any section placed in ".text".

Signed-off-by: Kevin O'Connor 
Signed-off-by: Roger Pau Monné 
Reported by: Ed Maste 

diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index b976fb0..6616721 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -34,18 +34,22 @@ COMMONTRAILER = """
 # Determine section locations
 ##
 
-# Align 'pos' to 'alignbytes' offset
+# Align 'pos' up to 'alignbytes' offset
 def alignpos(pos, alignbytes):
 mask = alignbytes - 1
 return (pos + mask) & ~mask
 
+# Align 'pos' down to 'alignbytes' offset
+def aligndown(pos, alignbytes):
+mask = alignbytes - 1
+return pos & ~mask
+
 # Determine the final addresses for a list of sections that end at an
 # address.
 def setSectionsStart(sections, endaddr, minalign=1, segoffset=0):
 totspace = 0
 for section in sections:
-if section.align > minalign:
-minalign = section.align
+minalign = max(minalign, section.align)
 totspace = alignpos(totspace, section.align) + section.size
 startaddr = int((endaddr - totspace) / minalign) * minalign
 curaddr = startaddr
@@ -269,7 +273,7 @@ def doLayout(sections, config, genreloc):
 final_sec32low_end = BUILD_LOWRAM_END
 zonelow_base = final_sec32low_end - 64*1024
 relocdelta = final_sec32low_end - sec32low_end
-li.sec32low_start, li.sec32low_align = setSectionsStart(
+li.sec32low_start, sec32low_align = setSectionsStart(
 sections32low, sec32low_end, 16
 , segoffset=zonelow_base - relocdelta)
 li.sec32low_end = sec32low_end
@@ -405,6 +409,8 @@ def writeLinkerScripts(li, out16, out32seg, out32flat):
 if li.config.get('CONFIG_MULTIBOOT'):
 multiboot_header = "LONG(0x1BADB002) LONG(0) LONG(-0x1BADB002)"
 sec32all_start -= 3 * 4
+sec32all_align = max([section.align for section in li.sections])
+sec32all_start = aligndown(sec32all_start, sec32all_align)
 out += outXRefs(filesections32flat, exportsyms=[li.entrysym]) + """
 _reloc_min_align = 0x%x ;
 zonefseg_start = 0x%x ;

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] AHCI: Set transfer mode according to the capabilities of connected drive

[Kevin O'Connor]

On Mon, Feb 15, 2016 at 12:00:48PM +, Zeh, Werner wrote:
> Hi Kevin.
> 
> We had some issues with some connected AHCI devices in SeaBIOS. We
> have connected some CF-Cards by using a simple SATA<->IDE bridge to
> the mainboard and in some cases, the drive (which is the CF-card)
> was not recognized correctly. After some deeper analysis we found
> that SeaBIOS does not set up the transfer rate which is used to
> communicate to the drive. The supported transfer rate can be found
> in the data structure which is delivered by IDENTIFY_DEVICE command.
> 
> So in our error cases the default transfer rate was too high and
> therefore data error has occurred.  I have attached a patch which
> will deal with this case on AHCI controllers. Maybe you can push
> this patch to mainline or at least have a look at it.
> 
> Up to now I have verified the function of this patch with the latest
> master branch of SeaBIOS and a Broadwell-DE CPU.  I have used PIO4,
> default PIO, Multiword-DMA2 and several Ultra-DMA CF-cards to ensure
> that all three paths work properly.
> 
> BTW: I am not that familiar with code style in SeaBIOS. If I made
> some formal mistakes, feel free to correct them.

Thanks.  I'm not that familiar with the AHCI internals.  Gerd, would
you be able to review?

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH 1/2] tpm: Unify tpm_fill_hash()/tpm_log_extend_event() and use in BIOS interface

[Kevin O'Connor]

On Mon, Feb 08, 2016 at 07:19:27AM -0500, Stefan Berger wrote:
> On 02/06/2016 01:35 PM, Kevin O'Connor wrote:
> >Don't call tpm_fill_hash() or tpm_log_extend_event() from any internal
> >code (ie, tpm_add_measurement_to_log).  The internal code does not
> >require the additional checks that these functions provide.
> >
> >Unify the tpm_fill_hash() and tpm_log_extend_event() into a new
> >function hash_log_extend(), and use this function only in the 16bit
> >BIOS interface code.  With the code now specific to the BIOS interface
> >it can more easily return a BIOS specific error return code.
[...]
> 
> ACK.

Thanks - I pushed this change.  (I did not push patch 2.)

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] Error while compiling Seabios for Arch Linux ARM

[Kevin O'Connor]

On Thu, Feb 18, 2016 at 05:49:39PM +0200, XJDHDR wrote:
> >> Greetings
> >> 
> >> I am currently trying to compile Seabios for Arch Linux ARM and I've
> >> encountered an error during the compilation ‎
> >
> >SeaBIOS implements an x86 legacy BIOS. As such, it requires an x86
> >compiler. So, you need to either compile on an x86 machine or setup
> >and use an x86 cross compilation toolchain.
> >
> >-Kevin
> 
> Thank you for the response Kevin. Correct me if I'm wrong but won't x86 
> binaries not work on an ARM system as ARM processors don't understand x86. 
> 
> I am attempting to get QEMU compiled and working on my ARM system and SeaBIOS 
> is a dependency ‎for QEMU, hence my need to have the former compiled for ARM. 
> Do you know if QEMU on ARM is okay with SeaBIOS' binaries being x86? If so, I 
> think this will make things a lot easier for me. 
> 

QEMU uses SeaBIOS when it emulates an x86 machine.  SeaBIOS is always
compiled with an x86 compiler.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios