From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45448) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X6dYw-0002ox-Tx for qemu-devel@nongnu.org; Mon, 14 Jul 2014 06:32:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X6dYq-0002cg-7X for qemu-devel@nongnu.org; Mon, 14 Jul 2014 06:32:50 -0400 Message-ID: <53C3B1C9.9000106@suse.de> Date: Mon, 14 Jul 2014 12:32:41 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1405268253-33465-1-git-send-email-agraf@suse.de> <1405268253-33465-4-git-send-email-agraf@suse.de> <53C37B86.20904@redhat.com> In-Reply-To: <53C37B86.20904@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/5] PPC: mac_nvram: Allow 2 and 4 byte accesses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-ppc@nongnu.org Cc: programmingkidx@gmail.com, mark.cave-ayland@ilande.co.uk, qemu-devel@nongnu.org On 14.07.14 08:41, Paolo Bonzini wrote: > Il 13/07/2014 18:17, Alexander Graf ha scritto: >> The NVRAM in our Core99 machine really supports 2byte and 4byte accesses >> just as well as 1byte accesses. In fact, Mac OS X uses those. >> >> Add support for higher register size granularities. >> >> Signed-off-by: Alexander Graf >> --- >> hw/nvram/mac_nvram.c | 43 ++++++++++++++++++++++++++++++++----------- >> 1 file changed, 32 insertions(+), 11 deletions(-) >> >> diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c >> index bcff074..0a6df44 100644 >> --- a/hw/nvram/mac_nvram.c >> +++ b/hw/nvram/mac_nvram.c >> @@ -40,32 +40,53 @@ >> #define DEF_SYSTEM_SIZE 0xc10 >> >> /* macio style NVRAM device */ >> -static void macio_nvram_writeb(void *opaque, hwaddr addr, >> - uint64_t value, unsigned size) >> +static void macio_nvram_write(void *opaque, hwaddr addr, >> + uint64_t value, unsigned size) >> { >> MacIONVRAMState *s = opaque; >> >> addr = (addr >> s->it_shift) & (s->size - 1); >> - s->data[addr] = value; >> - NVR_DPRINTF("writeb addr %04" PHYS_PRIx " val %" PRIx64 "\n", >> addr, value); >> + switch (size) { >> + case 1: >> + s->data[addr] = value; >> + break; >> + case 2: >> + stw_be_p(&s->data[addr], value); >> + break; >> + case 4: >> + stl_be_p(&s->data[addr], value); >> + break; >> + } >> + NVR_DPRINTF("write%d addr %04" PRIx64 " val %" PRIx64 "\n", >> size, addr, >> + value); >> } >> >> -static uint64_t macio_nvram_readb(void *opaque, hwaddr addr, >> - unsigned size) >> +static uint64_t macio_nvram_read(void *opaque, hwaddr addr, >> + unsigned size) >> { >> MacIONVRAMState *s = opaque; >> - uint32_t value; >> + uint32_t value = 0; >> >> addr = (addr >> s->it_shift) & (s->size - 1); >> - value = s->data[addr]; >> - NVR_DPRINTF("readb addr %04x val %x\n", (int)addr, value); >> + switch (size) { >> + case 1: >> + value = s->data[addr]; >> + break; >> + case 2: >> + value = lduw_be_p(&s->data[addr]); >> + break; >> + case 4: >> + value = ldl_be_p(&s->data[addr]); >> + break; >> + } >> + NVR_DPRINTF("read%d addr %04x val %x\n", size, (int)addr, value); >> >> return value; >> } >> >> static const MemoryRegionOps macio_nvram_ops = { >> - .read = macio_nvram_readb, >> - .write = macio_nvram_writeb, >> + .read = macio_nvram_read, >> + .write = macio_nvram_write, >> .endianness = DEVICE_BIG_ENDIAN, >> }; >> >> > > I think this ought to be enough: > > static const MemoryRegionOps test_ioport_byte_ops = { > .read = macio_nvram_readb, > .write = macio_nvram_writeb, > + .valid.min_access_size = 1, > + .valid.max_access_size = 4, > + .impl.min_access_size = 1, > + .impl.max_access_size = 1, > .endianness = DEVICE_BIG_ENDIAN, > }; Heh - I knew there had to be a trick to this :). That's certainly a lot cleaner. Alex