From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KY1gW-00057H-Sf for qemu-devel@nongnu.org; Tue, 26 Aug 2008 12:46:24 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KY1gU-00056b-Vb for qemu-devel@nongnu.org; Tue, 26 Aug 2008 12:46:24 -0400 Received: from [199.232.76.173] (port=33823 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KY1gU-00056V-Qp for qemu-devel@nongnu.org; Tue, 26 Aug 2008 12:46:22 -0400 Received: from wf-out-1314.google.com ([209.85.200.168]:11133) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KY1gU-00007a-D5 for qemu-devel@nongnu.org; Tue, 26 Aug 2008 12:46:22 -0400 Received: by wf-out-1314.google.com with SMTP id 27so2381178wfd.4 for ; Tue, 26 Aug 2008 09:46:20 -0700 (PDT) Message-ID: Date: Tue, 26 Aug 2008 19:46:20 +0300 From: "Blue Swirl" Subject: Re: [Qemu-devel] [PATCH v2 1/6] Use IO port for qemu<->guest BIOS communication. In-Reply-To: <20080826082453.GV6192@minantech.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20080825095800.18703.30602.stgit@gleb-debian.qumranet.com.qumranet.com> <20080825095805.18703.63202.stgit@gleb-debian.qumranet.com.qumranet.com> <48B2C0A1.7040309@codemonkey.ws> <20080825144026.GQ6192@minantech.com> <48B2F373.1020606@codemonkey.ws> <20080826082453.GV6192@minantech.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gleb Natapov Cc: qemu-devel@nongnu.org On 8/26/08, Gleb Natapov wrote: > On Mon, Aug 25, 2008 at 09:27:18PM +0300, Blue Swirl wrote: > > > Blue Swirl: What do you think of switching sparc to use a structure more > > > like this? I do prefer a key-value mechanism verses a blob. Even with pure > > > MMIO, the same results could be achieved by treating the MMIO region as > > > registers and using a selector. > > > > I could switch Sparc to something like this, if the goal is that it > > will be used by all targets. > > > > There should be a .h file which lists the keys and which can be > > included from C and asm, like current firmware_abi.h. I'd define an > > offset (0x8000?) for architecture-specific keys which need not be in > > the same .h file. > > > Is the patch below what you mean? (not tested, but compiles) Yes, but I'd still put the code from the .h file and pc.c to a new .c file, only the keys and function prototypes to .h. > + int arch = key & FW_CFG_ARCH_LOCAL; > + > + key &= (~FW_CFG_ARCH_LOCAL); > + > + if (key >= FW_CFG_MAX_ENTRY) > + return 0; I was actually not thinking to use the arch part like this, but in your solution, arch part storage is handled same way as others. Great! > +static FWCfgState *bios_params; Now that the _read and _write functions use the opaque argument, this is not needed once allocated. > +static uint32_t bios_cfg_read(void *opaque, uint32_t addr) > +{ > > + FWCfgState *s = opaque; > + > + return firmware_cfg_read(s); > > +} > + > +static void bios_cfg_write(void *opaque, uint32_t addr, uint32_t value) > +{ > > + FWCfgState *s = opaque; > + > + firmware_cfg_select(s, (uint16_t)value); > +} > + I'd put these to the new .c file. One problem with this IO method is that it is not SMP safe, because of the cur_entry selection and reading will not be atomic. Think of boot CPU waking up all other CPUs with broadcast interrupt and then these other CPUs all want to know the boot parameters at the same time. The same could happen with warm reset. Directly mapped ROM doesn't have this problem, but multiplexed ROM (or Sparc64 nvram used now) is also unsafe. I could think of a couple of solutions: - different IO port for each CPU (not possible on PC) - per-CPU cur_entry, the device "knows" the CPU index (not very beautiful) - lock mechanism: before access, the CPU must win lock and only then it can access the data. But IO ports don't support atomic instructions? How about some kind of MAC-like protocol? It gets pretty complex. Are there other solutions?