Re: [SeaBIOS] [PATCH 3/3] qemu: add qemu ramfb support

[Kevin O'Connor]

On Wed, Jun 13, 2018 at 10:51:57AM +0200, Gerd Hoffmann wrote:
> Add support for qemu ramfb.  This is a simple boot framebuffer device,
> with normal ram being used to back the framebuffer and fw_cfg being used
> to configure the device.
>
> Use case (on x86): boot display for vgpu devices (which neither emulate
> vga nor have a vgabios).

[...]
> --- /dev/null
> +++ b/vgasrc/ramfb.c
> @@ -0,0 +1,123 @@
> +#include "biosvar.h" // GET_BDA
> +#include "output.h" // dprintf
> +#include "string.h" // memset16_far
> +#include "vgautil.h" // VBE_total_memory
> +#include "std/pmm.h" // struct pmmheader
> +#include "byteorder.h"
> +#include "fw/paravirt.h"
>
> +#define FRAMEBUFFER_WIDTH      1024
> +#define FRAMEBUFFER_HEIGHT     768
> +#define FRAMEBUFFER_BPP        4
> +#define FRAMEBUFFER_STRIDE     (FRAMEBUFFER_BPP * FRAMEBUFFER_WIDTH)
> +#define FRAMEBUFFER_SIZE       (FRAMEBUFFER_STRIDE * FRAMEBUFFER_HEIGHT)
>
> +struct QemuRAMFBCfg {
> u64 addr;
> u32 fourcc;
> u32 flags;
> u32 width;
> u32 height;
> u32 stride;
>
> +};
>
> +#define fourcc_code(a, b, c, d) ((u32)(a) | ((u32)(b) << 8) | \
>                             ((u32)(c) << 16) | ((u32)(d) << 24))
>
> +#define DRM_FORMAT_RGB565       fourcc_code('R', 'G', '1', '6') /* [15:0] R:G:B 5:6:5 little endian */
> +#define DRM_FORMAT_RGB888       fourcc_code('R', 'G', '2', '4') /* [23:0] R:G:B little endian */
> +#define DRM_FORMAT_XRGB8888     fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 little endian */
>
> +static u32
> +allocate_framebuffer(void)
> +{
> u32 pmmscan;
> for (pmmscan=0; pmmscan < BUILD_BIOS_SIZE; pmmscan+=16) {
>    struct pmmheader *pmm = (void*)pmmscan;
>    if (GET_FARVAR(SEG_BIOS, pmm->signature) != PMM_SIGNATURE)
>        continue;
>    if (checksum_far(SEG_BIOS, pmm, GET_FARVAR(SEG_BIOS, pmm->length)))
>        continue;
>    struct segoff_s entry = GET_FARVAR(SEG_BIOS, pmm->entry);
>    dprintf(1, "ramfb: alloc framebuffer via pmm call to %04x:%04x\n"
>            , entry.seg, entry.offset);
>    u16 res1, res2;
>    asm volatile(
>        "pushl %0\n"
>        "pushw $(8|2)\n"            // Permanent high memory request
>        "pushl $0xffffffff\n"       // Anonymous handle
>        "pushl $" __stringify(FRAMEBUFFER_SIZE / 16) "\n"
>        "pushw $0x00\n"             // PMM allocation request
>        "lcallw *12(%%esp)\n"
>        "addl $16, %%esp\n"
>        "cli\n"
>        "cld\n"
>        : "+r" (entry.segoff), "=a" (res1), "=d" (res2) : : "cc", "memory");
>    u32 res = res1 | (res2 << 16);
>    if (!res || res == PMM_FUNCTION_NOT_SUPPORTED)
>        return 0;
>    dprintf(1, "ramfb: framebuffer allocated at %x\n", res);
>    return res;
> }
> return 0;
>
> +}

It would be preferable to re-factor allocate_extra_stack() so there is
just one pmm allocation function.
> +int
> +ramfb_setup(void)
> +{
> dprintf(1, "ramfb: init\n");
> if (GET_GLOBAL(HaveRunInit))
>    return 0;
> u32 count;
> qemu_cfg_read_entry(&count, QEMU_CFG_FILE_DIR, sizeof(count));
> count = be32_to_cpu(count);
> u32 select, e, fb;
> for (select = 0, e = 0; e < count; e++) {
>    struct QemuCfgFile qfile;
>    qemu_cfg_read(&qfile, sizeof(qfile));
>    if (memcmp_far(GET_SEG(SS), qfile.name,
>                   GET_SEG(CS), "etc/ramfb", 10) == 0)
>        select = be16_to_cpu(qfile.select);
> }

Could this be moved into a new qemu_cfg_scan_file() function in
paravirt.c, and thus avoid patch 2?
> if (select == 0) {
>    dprintf(1, "ramfb: fw_cfg (etc/ramfb) file not found\n");
>    return -1;
> }
> dprintf(1, "ramfb: fw_cfg (etc/ramfb) file at slot 0x%x\n", select);
> fb = allocate_framebuffer();
> if (!fb) {
>    dprintf(1, "ramfb: allocating framebuffer failed\n");
>    return -1;
> }
> u64 addr = fb;
> u8 bpp = FRAMEBUFFER_BPP * 8;
> u32 xlines = FRAMEBUFFER_WIDTH;
> u32 ylines = FRAMEBUFFER_HEIGHT;
> u32 linelength = FRAMEBUFFER_STRIDE;
> dprintf(1, "Found FB @ %llx %dx%d with %d bpp (%d stride)\n"
>        , addr, xlines, ylines, bpp, linelength);
> if (!addr || addr > 0xffffffff
>    || (bpp != 15 && bpp != 16 && bpp != 24 && bpp != 32)) {
>    dprintf(1, "Unable to use FB\n");
>    return -1;
> }
> cbvga_setup_modes(addr, bpp, xlines, ylines, linelength);
> struct QemuRAMFBCfg cfg = {
>    .addr   = cpu_to_be64(fb),
>    .fourcc = cpu_to_be32(DRM_FORMAT_XRGB8888),
>    .flags  = cpu_to_be32(0),
>    .width  = cpu_to_be32(FRAMEBUFFER_WIDTH),
>    .height = cpu_to_be32(FRAMEBUFFER_HEIGHT),
>    .stride = cpu_to_be32(FRAMEBUFFER_STRIDE),
> };
> qemu_cfg_write_entry(&cfg, select, sizeof(cfg));
> return 0;
>
> +}
> diff --git a/vgasrc/Kconfig b/vgasrc/Kconfig
> index 4443c0b37a..1933b0e23f 100644
> --- a/vgasrc/Kconfig
> +++ b/vgasrc/Kconfig
> @@ -70,6 +70,13 @@ menu "VGA ROM"
>                  v3.0+. The vgabios works with the qemu stdvga too (use
>                  "qemu -device VGA,romfile=/path/to/vgabios.bin")".
>    config VGA_RAMFB
>        depends on QEMU
>        bool "qemu ram framebuffer (experimental)"
>        select VGA_EMULATE_TEXT
>        help
>            qemu ram framebuffer (experimental)

I see it is marked as experimental - is this a proof of concept patch
or is this feature ready for a merge into qemu and seabios?

Thanks,
-Kevin

[PATCH 3/3] qemu: add qemu ramfb support

[Gerd Hoffmann]

Hi,
> It would be preferable to re-factor allocate_extra_stack() so there is
> just one pmm allocation function.

I'll try.
> > qemu_cfg_read_entry(&count, QEMU_CFG_FILE_DIR, sizeof(count));
> > count = be32_to_cpu(count);
> > u32 select, e, fb;
> > for (select = 0, e = 0; e < count; e++) {
> >    struct QemuCfgFile qfile;
> >    qemu_cfg_read(&qfile, sizeof(qfile));
> >    if (memcmp_far(GET_SEG(SS), qfile.name,
> >                   GET_SEG(CS), "etc/ramfb", 10) == 0)
> >        select = be16_to_cpu(qfile.select);
> > }
>
> Could this be moved into a new qemu_cfg_scan_file() function in
> paravirt.c, and thus avoid patch 2?

Can't be avoided completely ...
> > qemu_cfg_write_entry(&cfg, select, sizeof(cfg));

... because of this.  But, yes, should make the patch smaller and keep
more bits private to paravirt.c

I'll check this out.
> >    config VGA_RAMFB
> >        depends on QEMU
> >        bool "qemu ram framebuffer (experimental)"
> >        select VGA_EMULATE_TEXT
> >        help
> >            qemu ram framebuffer (experimental)
>
> I see it is marked as experimental - is this a proof of concept patch
> or is this feature ready for a merge into qemu and seabios?

Oh, the experimental can be dropped now, will fix that.  qemu patches
should be merged soon.

cheers,
  Gerd