From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NRTEw-000190-Nn for qemu-devel@nongnu.org; Sun, 03 Jan 2010 11:23:38 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NRTEr-0000zm-Ch for qemu-devel@nongnu.org; Sun, 03 Jan 2010 11:23:38 -0500 Received: from [199.232.76.173] (port=37381 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NRTEr-0000zX-4G for qemu-devel@nongnu.org; Sun, 03 Jan 2010 11:23:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43628) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NRTEq-0001tk-MD for qemu-devel@nongnu.org; Sun, 03 Jan 2010 11:23:33 -0500 Date: Sun, 3 Jan 2010 18:20:28 +0200 From: "Michael S. Tsirkin" Message-ID: <20100103162028.GE8137@redhat.com> References: <1262483450-15206-1-git-send-email-agraf@suse.de> <1262483450-15206-3-git-send-email-agraf@suse.de> <20100103153215.GA8137@redhat.com> <20100103154726.GC8137@redhat.com> <3E418050-4BEB-4583-86B6-C47F2D1B8649@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3E418050-4BEB-4583-86B6-C47F2D1B8649@suse.de> Subject: [Qemu-devel] Re: [PATCH 2/6] Add config space conversion function for uni_north List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Blue Swirl , QEMU Developers , Aurelien Jarno On Sun, Jan 03, 2010 at 05:13:12PM +0100, Alexander Graf wrote: > > On 03.01.2010, at 16:47, Michael S. Tsirkin wrote: > > > On Sun, Jan 03, 2010 at 04:40:12PM +0100, Alexander Graf wrote: > >> > >> On 03.01.2010, at 16:32, Michael S. Tsirkin wrote: > >> > >>> On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote: > >>>> As stated in the previous patch, the Uninorth PCI bridge requires different > >>>> layouts in its PCI config space accessors. > >>>> > >>>> This patch introduces a conversion function that makes it compatible with > >>>> the way Linux accesses it. > >>>> > >>>> I also kept an OpenBIOS compatibility hack in. I think it'd be better to > >>>> take small steps here and do the config space access rework in OpenBIOS > >>>> later on. When that's done we can remove that hack. > >>>> > >>>> Signed-off-by: Alexander Graf > >>>> --- > >>>> hw/unin_pci.c | 35 +++++++++++++++++++++++++++++++++++ > >>>> 1 files changed, 35 insertions(+), 0 deletions(-) > >>>> > >>>> diff --git a/hw/unin_pci.c b/hw/unin_pci.c > >>>> index fdb9401..1c49008 100644 > >>>> --- a/hw/unin_pci.c > >>>> +++ b/hw/unin_pci.c > >>>> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque) > >>>> { > >>>> } > >>>> > >>>> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr) > >>>> +{ > >>>> + uint32_t retval; > >>>> + uint32_t reg = s->config_reg; > >>>> + > >>>> + if (reg & (1u << 31)) { > >>>> + /* XXX OpenBIOS compatibility hack */ > >>>> + retval = reg; > >>>> + addr |= reg & 7; > >>>> + } else if (reg & 1) { > >>>> + /* Set upper valid bit and remove lower one */ > >>>> + retval = (reg & ~3u) | (1u << 31); > >>>> + } else { > >>>> + uint32_t slot, func; > >>>> + uint32_t devfn; > >>>> + > >>>> + /* Grab CFA0 style values */ > >>>> + slot = ffs(reg & 0xfffff800) - 1; > >>>> + func = (reg >> 8) & 7; > >>>> + devfn = PCI_DEVFN(slot, func); > >>>> + > >>>> + /* ... and then convert them to x86 format */ > >>>> + retval = (reg & 0xfc) | (devfn << 8) | (1u << 31); > >>> > >>> Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit > >>> number? This way this encoding can be changed down the road. > >> > >> I don't think I understand this comment? :-) > > > > This puts reg+dev+fn in a format that pci_host can the understand > > correct? So it would make sense to have an inline function > > in pci host that gets 3 parameters and does the encoding. > > We're doing the reverse here. We get a uint32_t (host->config_reg) and need to do something with it. > > We could either call a helper that splits it into bus,dev,fn or we could just put all of them into a single uint32_t again that later on gets interpreted in a specified format. > > I figured I'd have to touch less code and keep things more stable for the other (non-uninorth) buses if I keep the x86 format as "default format" and just convert to it. Passing a uint32_t is also easier than passing 3 ints :-). > > Alex So what the comment above suggests, is adding helper routine that gets register device, function and creates 32 bit value in "default format". -- MST