From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40597) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGbVn-0006JD-KM for qemu-devel@nongnu.org; Mon, 02 Sep 2013 17:18:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VGbVi-0008Bj-1i for qemu-devel@nongnu.org; Mon, 02 Sep 2013 17:18:15 -0400 Received: from smtp1-g21.free.fr ([2a01:e0c:1:1599::10]:40311) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGbVh-0008Ag-Bx for qemu-devel@nongnu.org; Mon, 02 Sep 2013 17:18:09 -0400 Message-ID: <52250089.3010901@reactos.org> Date: Mon, 02 Sep 2013 23:18:01 +0200 From: =?UTF-8?B?SGVydsOpIFBvdXNzaW5lYXU=?= MIME-Version: 1.0 References: <1377283973-9320-1-git-send-email-hpoussin@reactos.org> <1377283973-9320-4-git-send-email-hpoussin@reactos.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/3] prep: improve Raven PCI host emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: =?UTF-8?B?QW5kcmVhcyBGw6RyYmU=?= =?UTF-8?B?cg==?= , QEMU Developers Peter Maydell a =C3=A9crit : > On 23 August 2013 19:52, Herv=C3=A9 Poussineau w= rote: >=20 >> - let it load a firmware (raw or elf image) >> - add a GPIO to let it handle the non-contiguous I/O address space >> - provide a bus master address space >=20 >> Also move isa_mem_base from PCI host to machine board. >=20 >> Simplify prep board code by relying on Raven PCI host to handle >> non-contiguous I/O, and to load BIOS (with a small hack required >> for Open Hack'Ware). >=20 > That seems like quite a lot to be doing in a single patch. Does > any of it split out for easier code review? Yes, v2 is in preparation. This patch will be split in multiple patches. >=20 >> +static uint64_t prep_io_read(void *opaque, hwaddr addr, >> + unsigned int size) >> +{ >> + PREPPCIState *s =3D opaque; >> + uint8_t buf[4]; >> + uint64_t val; >> + >> + if (s->contiguous_map =3D=3D 0) { >> + /* 64 KB contiguous space for IOs */ >> + addr &=3D 0xFFFF; >> + } else { >> + /* 8 MB non-contiguous space for IOs */ >> + addr =3D (addr & 0x1F) | ((addr & 0x007FFF000) >> 7); >> + } >> + >> + address_space_read(&s->pci_io_as, addr + 0x80000000, buf, size); >> + memcpy(&val, buf, size); >> + return val; >> +} >=20 > Since this is just forwarding accesses to another address > space, I'm fairly sure you could do it with a suitable collection > of alias and container memory regions. Yes, aliases should probably work, but it won't be handy to create lots=20 of them. Moreover, this function needs to be expanded later to handle an=20 additional endianness switch, which will change both addresses and values= ... >=20 >> + >> +static void prep_io_write(void *opaque, hwaddr addr, >> + uint64_t val, unsigned int size) >> +{ >> + PREPPCIState *s =3D opaque; >> + uint8_t buf[4]; >> + >> + if (s->contiguous_map =3D=3D 0) { >> + /* 64 KB contiguous space for IOs */ >> + addr &=3D 0xFFFF; >> + } else { >> + /* 8 MB non-contiguous space for IOs */ >> + addr =3D (addr & 0x1F) | ((addr & 0x007FFF000) >> 7); >> + } >> + >> + memcpy(buf, &val, size); >> + address_space_write(&s->pci_io_as, addr + 0x80000000, buf, size); >> +} >=20 > ...if you don't do it that way, please at least factor out the > address conversion code from the read and write routines. ... So, yes, I'll put conversion code in a common routine. Herv=C3=A9