On 07/14/17 20:09, Eduardo Habkost wrote:
> On Fri, Jul 14, 2017 at 10:40:08AM +0100, Mark Cave-Ayland wrote:
>> By exposing FWCfgIoState and FWCfgMemState internals we allow the possibility
>> for the internal MemoryRegion fields to be mapped by name for boards that
>> wish
>> to wire up the fw_cfg device themselves.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
>> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
> [...]
>> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
>> index b980cba..b77ea48 100644
>> --- a/include/hw/nvram/fw_cfg.h
>> +++ b/include/hw/nvram/fw_cfg.h
>> @@ -1,8 +1,19 @@
>> #ifndef FW_CFG_H
>> #define FW_CFG_H
>>
>> +#include "qemu/typedefs.h"
>> #include "exec/hwaddr.h"
>> #include "hw/nvram/fw_cfg_keys.h"
>> +#include "hw/sysbus.h"
>> +#include "sysemu/dma.h"
>> +
>> +#define TYPE_FW_CFG "fw_cfg"
>> +#define TYPE_FW_CFG_IO "fw_cfg_io"
>> +#define TYPE_FW_CFG_MEM "fw_cfg_mem"
>> +
>> +#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG)
>> +#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO)
>> +#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM)
>>
>> typedef struct FWCfgFile {
>> uint32_t size; /* file size */
>> @@ -35,6 +46,45 @@ typedef struct FWCfgDmaAccess {
>>
>> typedef void (*FWCfgReadCallback)(void *opaque);
>>
>> +struct FWCfgState {
>> + /*< private >*/
>> + SysBusDevice parent_obj;
>> + /*< public >*/
>> +
>> + uint16_t file_slots;
>> + FWCfgEntry *entries[2];
>> + int *entry_order;
>> + FWCfgFiles *files;
>> + uint16_t cur_entry;
>> + uint32_t cur_offset;
>> + Notifier machine_ready;
>> +
>> + int fw_cfg_order_override;
>> +
>> + bool dma_enabled;
>> + dma_addr_t dma_addr;
>> + AddressSpace *dma_as;
>> + MemoryRegion dma_iomem;
>> +};
>> +
>> +struct FWCfgIoState {
>> + /*< private >*/
>> + FWCfgState parent_obj;
>> + /*< public >*/
>> +
>> + MemoryRegion comb_iomem;
>> +};
>> +
>> +struct FWCfgMemState {
>> + /*< private >*/
>> + FWCfgState parent_obj;
>> + /*< public >*/
>> +
>> + MemoryRegion ctl_iomem, data_iomem;
>> + uint32_t data_width;
>> + MemoryRegionOps wide_data_ops;
>> +};
>
> Why do you need the full struct declaration to be exposed in the
> header?
Different board code wants to hook up "comb_iomem" manually to different
address spaces, so they need to access the field directly. This is the
ultimate goal of the entire exercise, IIRC.
> The memory regions are supposed to be visible as QOM
> children to the fw_cfg device, already.
I don't understand this. How else can board code work with "comb_iomem"
than described above? If there is a way, I agree it would be preferable.
Thanks
Laszlo
