From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55121) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW65A-0002cO-DT for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:35:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YW656-0005ya-CL for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:35:36 -0400 Received: from mail-wg0-x22d.google.com ([2a00:1450:400c:c00::22d]:46071) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW655-0005w9-Rf for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:35:32 -0400 Received: by wggx13 with SMTP id x13so17563634wgg.12 for ; Thu, 12 Mar 2015 09:35:31 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5501C050.9040308@redhat.com> Date: Thu, 12 Mar 2015 17:35:28 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <20150311152758.GU1832@HEDWIG.INI.CMU.EDU> <55008E80.50907@redhat.com> <20150311204537.GW1832@HEDWIG.INI.CMU.EDU> <5500B3E1.8020402@redhat.com> <55019FE8.1020706@redhat.com> <5501B3E1.5000208@redhat.com> <20150312161631.GX1832@HEDWIG.INI.CMU.EDU> In-Reply-To: <20150312161631.GX1832@HEDWIG.INI.CMU.EDU> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] fw_cfg specification ? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gabriel L. Somlo" , Laszlo Ersek Cc: jordan.l.justen@intel.com, gleb@cloudius-systems.com, qemu-devel@nongnu.org On 12/03/2015 17:16, Gabriel L. Somlo wrote: > static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr, > unsigned size, bool is_write) > { > +//FIXME: additional checks before we nuke fw_cfg_data_mem_write() ? yes, "&& !is_write". Or just leave an empty fw_cfg_data_mem_write. > return addr == 0; > } > > @@ -336,7 +314,7 @@ static void fw_cfg_comb_write(void *opaque, hwaddr addr, > { > switch (size) { > case 1: > - fw_cfg_write(opaque, (uint8_t)value); > + //FIXME: unused, fix fw_cfg_comb_mem_ops then remove case > break; > case 2: > fw_cfg_select(opaque, (uint16_t)value); > @@ -347,6 +325,7 @@ static void fw_cfg_comb_write(void *opaque, hwaddr addr, > static bool fw_cfg_comb_valid(void *opaque, hwaddr addr, > unsigned size, bool is_write) > { > +//FIXME: update checks before removing size==1 fw_cfg_comb_write() case > return (size == 1) || (is_write && size == 2); Same here: "size == 1 && !is_write", or just leave an empty "case" after removing the call to fw_cfg_write. > } > > @@ -358,6 +337,7 @@ static const MemoryRegionOps fw_cfg_ctl_mem_ops = { > > static const MemoryRegionOps fw_cfg_data_mem_ops = { > .read = fw_cfg_data_mem_read, > +//FIXME: nuke this after updating fw_cfg_data_mem_valid() If you want, but it's not necessary. Paolo > .write = fw_cfg_data_mem_write, > .endianness = DEVICE_BIG_ENDIAN, > .valid = { > @@ -458,7 +438,6 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key, > s->entries[arch][key].data = data; > s->entries[arch][key].len = len; > s->entries[arch][key].callback_opaque = NULL; > - s->entries[arch][key].callback = NULL; > > return ptr; > } > @@ -502,23 +481,6 @@ void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value) > fw_cfg_add_bytes(s, key, copy, sizeof(value)); > } > > -void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback, > - void *callback_opaque, void *data, size_t len) > -{ > - int arch = !!(key & FW_CFG_ARCH_LOCAL); > - > - assert(key & FW_CFG_WRITE_CHANNEL); > - > - key &= FW_CFG_ENTRY_MASK; > - > - assert(key < FW_CFG_MAX_ENTRY && len <= UINT32_MAX); > - > - s->entries[arch][key].data = data; > - s->entries[arch][key].len = (uint32_t)len; > - s->entries[arch][key].callback_opaque = callback_opaque; > - s->entries[arch][key].callback = callback; > -} > - > void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, > FWCfgReadCallback callback, void *callback_opaque, > void *data, size_t len) > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h > index 6d8a8ac..b2e10c2 100644 > --- a/include/hw/nvram/fw_cfg.h > +++ b/include/hw/nvram/fw_cfg.h > @@ -69,8 +69,6 @@ void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value); > void fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value); > void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value); > void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value); > -void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback, > - void *callback_opaque, void *data, size_t len); > void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data, > size_t len); > void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, > >