From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nezsr-0000Yb-DF for qemu-devel@nongnu.org; Tue, 09 Feb 2010 18:52:45 -0500 Received: from [199.232.76.173] (port=46224 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nezsq-0000YD-UF for qemu-devel@nongnu.org; Tue, 09 Feb 2010 18:52:44 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Nezsn-0008EA-J0 for qemu-devel@nongnu.org; Tue, 09 Feb 2010 18:52:44 -0500 Received: from mail-iw0-f185.google.com ([209.85.223.185]:49094) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Nezsn-0008E6-8E for qemu-devel@nongnu.org; Tue, 09 Feb 2010 18:52:41 -0500 Received: by iwn15 with SMTP id 15so6038225iwn.19 for ; Tue, 09 Feb 2010 15:52:40 -0800 (PST) Message-ID: <4B71F545.2080001@codemonkey.ws> Date: Tue, 09 Feb 2010 17:52:37 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API References: <1265752899-26980-1-git-send-email-aliguori@us.ibm.com> <1265752899-26980-8-git-send-email-aliguori@us.ibm.com> <4B71E72C.8050702@codemonkey.ws> <4B71EB73.608@codemonkey.ws> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: malc Cc: Alex Graf , qemu-devel@nongnu.org, Michael Tsirkin On 02/09/2010 05:36 PM, malc wrote: > Let's see: > > Currently we have this > > > readb(...): > dostuff > return stuff > > readw(...): > dostuff > return stuff > And this is completely wrong. For the most part, device models don't consistently handle access to registers via their non native sizes. Some devices return the lower part of a dword register when accessed with a word accessor. Some devices only return the register when there's a dword access. What's worse, is that if a device only registers a byte accessor, because of the default accessors, a word access returns two registers but a dword access returns ~0U. Some device models actually rely on this behaviour. > You are replacing it with > > read(size...): > if (size == 1): do1 > elif (size == 2): do2 > else: # and here your code assumes that everything is handy dandy > # and size is 4 > do4 > This is ugly because it's a programmatic conversion. Once we have this API, we can switch to: read(addr, size...): switch(addr): case REG0: return s->reg0; case REG1: return s->reg1; Along with having a table like: pci_regs = { {REG0, DWORD}, {REG1, BYTE}, {REG2, BYTE}, {REG3, WORD} } That allows the PCI layer to invoke the callback and do the right operations to transparently handle the access size without the device model having to deal with it. The reason I've left size in the callback at all is that I suspect there will always be a device that behaves weirdly and needs to know about the accessor size. But for the most part, I think this model will make things much more consistent and eliminate a huge class of emulation bugs. We can even take it further, and do something like: pci_regs = { PCI_REG_DWORD(REG0, DeviceState, reg0), PCI_REG_BYTE(REG1, DeviceState, reg1), ... } In which case, we only need to handle the case in switch() if we need to implement a side effect of the register operation. But none of this is possible if we completely rely on every device to implement non-dword access in it's own broken way. Regards, Anthony Liguori