From patchwork Thu Jan 28 00:14:54 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kevin O'Connor X-Patchwork-Id: 574490 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 92D9B1402EC for ; Thu, 28 Jan 2016 11:15:18 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=koconnor-net.20150623.gappssmtp.com header.i=@koconnor-net.20150623.gappssmtp.com header.b=qI04lSRH; dkim-atps=neutral Received: from localhost ([::1]:53200 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOaF2-0004z4-Bj for incoming@patchwork.ozlabs.org; Wed, 27 Jan 2016 19:15:16 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45426) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOaEm-0004gh-0W for qemu-devel@nongnu.org; Wed, 27 Jan 2016 19:15:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aOaEi-0006T6-QC for qemu-devel@nongnu.org; Wed, 27 Jan 2016 19:14:59 -0500 Received: from mail-qg0-x22e.google.com ([2607:f8b0:400d:c04::22e]:35555) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOaEi-0006T0-Gm for qemu-devel@nongnu.org; Wed, 27 Jan 2016 19:14:56 -0500 Received: by mail-qg0-x22e.google.com with SMTP id o11so21413760qge.2 for ; Wed, 27 Jan 2016 16:14:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=koconnor-net.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=VNTNd+czBv9hun6SwWBJQ0N7KURY4WQeHZt6ly0A2Dw=; b=qI04lSRHnCqi7TcfYQYBJV6ORm1r7D7TXs4yKjgfFLXMboF1nnFLZEYm6yIg22yCTg XheNHFPCUzHgttUrqCOGxBQW8vqW9Oh7E2n1PlpnNQGyLauWNlFYkigUav5fK1Cfay2v DQqtEat6EGFcy65JGgCDnNk6RvbgxHXL6mN54WvQOrCDLC6TrQrvePA3CiFSysCdAzds wQeYbyJRvqZExlj2+pXQW11UGmtoh41BrzzrXYpp/6yB29ZuTkx7BQCSu8tkDQ0a1Fxd +RaxlT9nzPYpNVqE/UA9qAG7ZbFDUtfbJ+jeFIUm2ffUESNmZ1E1CxsLctJ3kk0MQvu3 tTfQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=VNTNd+czBv9hun6SwWBJQ0N7KURY4WQeHZt6ly0A2Dw=; b=mgi8pc3uLqNv1jS6afjyhgI1zePfo5ti9C0H3xWM45ukgSd3ASzpkwFzfpIzK4D8U6 qFGiaIEURVmHKJ+ixN6gASaL9l1ZGy390gW+J8dbhcB0Vlq/QZ4O3NBk42ZCh7I4t1fL CI06vHf1IGa2EMxycTGLs70MAuEv+b9ziB7p7r+RbdxWnluNNtYFNc8A72ysPle2HL4T pm8etRX6nWnkuLZTQXI2yvwPCTIdsYcjKW7TnO4U4vcRoz2ZeygGMlYj66s7zwOVsbv+ 4tqd/Xzny5T/5e/QnIikuFGhNcdeW3HjYPUaTb+R6cd9o9KLrVj1ZwGN6Uki3i0f61aG FO4A== X-Gm-Message-State: AG10YOS6W0RF5VLYCs3kbltod5B60iLgQJX64HhQuKYdIyMgQ0JpURdlJoWtT0Thz34v/Q== X-Received: by 10.140.28.161 with SMTP id 30mr129948qgz.36.1453940095874; Wed, 27 Jan 2016 16:14:55 -0800 (PST) Received: from localhost (209-122-232-221.c3-0.avec-ubr1.nyr-avec.ny.cable.rcn.com. [209.122.232.221]) by smtp.gmail.com with ESMTPSA id 107sm3705983qge.16.2016.01.27.16.14.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 27 Jan 2016 16:14:55 -0800 (PST) Date: Wed, 27 Jan 2016 19:14:54 -0500 From: Kevin O'Connor To: Marc =?iso-8859-1?Q?Mar=ED?= Message-ID: <20160128001454.GA27233@morn.lan> References: <1453727868-11147-1-git-send-email-markmb@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1453727868-11147-1-git-send-email-markmb@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:400d:c04::22e Cc: "Gabriel L. Somlo" , qemu-devel@nongnu.org, Gerd Hoffmann , Stefan Hajnoczi , Paolo Bonzini , Laszlo Subject: Re: [Qemu-devel] [PATCH v3] Add optionrom compatible with fw_cfg DMA version X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Mon, Jan 25, 2016 at 02:17:48PM +0100, Marc Marí wrote: > This optionrom is based on linuxboot.S. Hi Marc, Out of curiousity, how does the timing with this option rom compare to the previous SeaBIOS patches that implemented linux dma loading? When I first tried to compile this (on fc23), I got: In file included from /usr/include/features.h:389:0, from /usr/include/stdint.h:25, from /usr/lib/gcc/x86_64-redhat-linux/5.3.1/include/stdint.h:9, from linuxboot_dma.c:62: /usr/include/gnu/stubs.h:7:27: fatal error: gnu/stubs-32.h: No such file or directory compilation terminated. which I fixed by running "dnf install glibc-devel.i686". Is a configure check needed? See further comments below. [...] > --- /dev/null > +++ b/pc-bios/optionrom/linuxboot_dma.c > @@ -0,0 +1,262 @@ > +/* > + * Linux Boot Option ROM for fw_cfg DMA > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see . > + * > + * Copyright (c) 2015 Red Hat Inc. > + * Authors: Marc Marí > + */ > + > +asm( > +".text\n" > +".global _start\n" > +"_start:\n" > +" .short 0xaa55\n" > +" .byte (_end - _start) / 512\n" > +" lret\n" > +" .org 0x18\n" > +" .short 0\n" > +" .short _pnph\n" > +"_pnph:\n" > +" .ascii \"$PnP\"\n" > +" .byte 0x01\n" > +" .byte ( _pnph_len / 16 )\n" > +" .short 0x0000\n" > +" .byte 0x00\n" > +" .byte 0x00\n" > +" .long 0x00000000\n" > +" .short _manufacturer\n" > +" .short _product\n" > +" .long 0x00000000\n" > +" .short 0x0000\n" > +" .short 0x0000\n" > +" .short _bev\n" > +" .short 0x0000\n" > +" .short 0x0000\n" > +" .equ _pnph_len, . - _pnph\n" > +" .align 4, 0\n" > +"_bev:\n" > +".code16gcc\n" > +/* DS = CS */ > +" movw %cs, %ax\n" > +" movw %ax, %ds\n" > +" movl %esp, %ebp\n" > +"run_linuxboot:\n" > +" cli\n" > +" cld\n" > +" jmp load_kernel\n" > +); The run_linuxboot label doesn't seem to be used anywhere. [...] > +static inline uint16_t readw_addr32(const void *addr) { > + uint16_t val; > + asm("addr32 movw %1, %0" : "=r"(val) : "g"(addr)); > + barrier(); > + return val; > +} > + > +static inline uint32_t readl_addr32(const void *addr) { > + uint32_t val; > + asm("addr32 movl %1, %0" : "=r"(val) : "g"(addr)); > + barrier(); > + return val; > +} > + > +static inline void writel_addr32(void *addr, uint32_t val) { > + barrier(); > + asm("addr32 movl %0, %1" : : "r"(val), "g"(addr)); > +} The above does not look correct to me. Since the code is running in 16bit mode the above memory accesses are relative to the %ds segment. Because %ds=%cs this is going to access a different address than expected. What I think you want to do is assign %es=setup_addr>>4 and then perform the read at the given offset (eg, 0x206). [...] > +static void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len) > +{ > + FWCfgDmaAccess access; > + uint32_t control = (entry << 16) | BIOS_CFG_DMA_CTL_SELECT > + | BIOS_CFG_DMA_CTL_READ; > + > + access.address = cpu_to_be64((uint64_t)(uint32_t)buf); > + access.length = cpu_to_be32(len); > + access.control = cpu_to_be32(control); > + > + barrier(); > + > + outl(cpu_to_be32((uint32_t)&access), BIOS_CFG_DMA_ADDR_LOW); > + > + while(be32_to_cpu(access.control) & ~BIOS_CFG_DMA_CTL_ERROR) { > + barrier(); > + } > +} FYI, I think with a small incremental patch (see below) one could entirely replace the existing linuxboot.rom with your new code. The one caveat is that this patch requires that kvm support "big real mode" and I know there were quirks with that on some older Intel chips. However, I think the "insb" instruction would trap anyway, so maybe it's not an issue. -Kevin --- a/pc-bios/optionrom/linuxboot_dma.c +++ b/pc-bios/optionrom/linuxboot_dma.c @@ -73,6 +73,8 @@ asm( #define BIOS_CFG_DMA_CTL_SKIP 0x04 #define BIOS_CFG_DMA_CTL_SELECT 0x08 +#define BIOS_CFG_CTL 0x510 +#define BIOS_CFG_DATA 0x511 #define BIOS_CFG_DMA_ADDR_HIGH 0x514 #define BIOS_CFG_DMA_ADDR_LOW 0x518 @@ -87,6 +89,16 @@ typedef struct FWCfgDmaAccess { uint64_t address; } __attribute__((packed)) FWCfgDmaAccess; +static inline void outw(uint16_t value, uint16_t port) { + asm("outw %w0, %w1" : : "a"(value), "Nd"(port)); +} + +static inline uint32_t inl(uint16_t port) { + uint32_t value; + __asm__ __volatile__("inl %w1, %0" : "=a"(value) : "Nd"(port)); + return value; +} + static inline void outl(uint32_t value, uint16_t port) { asm("outl %0, %w1" : : "a"(value), "Nd"(port)); } @@ -124,6 +136,15 @@ static inline uint32_t be32_to_cpu(uint32_t x) { static void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len) { + if (inl(BIOS_CFG_DMA_ADDR_LOW) != 0x47464320) { + // Legacy PIO fw_cfg + outw(entry, BIOS_CFG_CTL); + asm volatile("movw %w0, %%es" :: "r"(0) : "memory"); + asm volatile("rep insb (%%dx), %%es:(%%edi)" + : "+c"(len), "+D"(buf) : "d"(BIOS_CFG_DATA) : "memory"); + return; + } + FWCfgDmaAccess access; uint32_t control = (entry << 16) | BIOS_CFG_DMA_CTL_SELECT | BIOS_CFG_DMA_CTL_READ;