sun4u: move fw_cfg/NVRAM devices to ebus PCI IO address space

[Qemu-devel] [PATCH 1/3] sun4u: switch to using qdev to instantiate fw_cfg interface

Posted by Mark Cave-Ayland 8 years, 4 months ago

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/sparc64/sun4u.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 69f565d..98ee6f5 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -512,7 +512,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                            graphic_width, graphic_height, graphic_depth,
                            (uint8_t *)&nd_table[0].macaddr);
 
-    fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+    dev = qdev_create(NULL, TYPE_FW_CFG_IO);
+    qdev_prop_set_uint32(dev, "iobase", BIOS_CFG_IOPORT);
+    qdev_prop_set_bit(dev, "dma_enabled", false);
+    qdev_init_nofail(dev);
+    s = SYS_BUS_DEVICE(dev);
+    memory_region_add_subregion(get_system_io(), BIOS_CFG_IOPORT,
+                                sysbus_mmio_get_region(s, 0));
+
+    fw_cfg = FW_CFG(dev);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 1/3] sun4u: switch to using qdev to instantiate fw_cfg interface

Posted by Philippe Mathieu-Daudé 8 years, 4 months ago

Hi Mark,

On 06/10/2017 10:00 AM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/sparc64/sun4u.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 69f565d..98ee6f5 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -512,7 +512,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>                             graphic_width, graphic_height, graphic_depth,
>                             (uint8_t *)&nd_table[0].macaddr);
>
> -    fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
> +    dev = qdev_create(NULL, TYPE_FW_CFG_IO);
> +    qdev_prop_set_uint32(dev, "iobase", BIOS_CFG_IOPORT);
> +    qdev_prop_set_bit(dev, "dma_enabled", false);
> +    qdev_init_nofail(dev);
> +    s = SYS_BUS_DEVICE(dev);
> +    memory_region_add_subregion(get_system_io(), BIOS_CFG_IOPORT,
> +                                sysbus_mmio_get_region(s, 0));

Now that you exported TYPE_FW_CFG_IO I think this might be 
useful/cleaner to have that code in an static inlined function in fw_cfg.h:

DeviceState *fw_cfg_create(uint32_t iobase[, bool dma_enabled]);

What do you think?

Anyway:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +
> +    fw_cfg = FW_CFG(dev);
>      fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
>      fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
>      fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
>

Re: [Qemu-devel] [PATCH 1/3] sun4u: switch to using qdev to instantiate fw_cfg interface

Posted by Mark Cave-Ayland 8 years, 4 months ago

On 10/06/17 18:55, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 06/10/2017 10:00 AM, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/sparc64/sun4u.c |   10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index 69f565d..98ee6f5 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -512,7 +512,15 @@ static void sun4uv_init(MemoryRegion
>> *address_space_mem,
>>                             graphic_width, graphic_height, graphic_depth,
>>                             (uint8_t *)&nd_table[0].macaddr);
>>
>> -    fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
>> +    dev = qdev_create(NULL, TYPE_FW_CFG_IO);
>> +    qdev_prop_set_uint32(dev, "iobase", BIOS_CFG_IOPORT);
>> +    qdev_prop_set_bit(dev, "dma_enabled", false);
>> +    qdev_init_nofail(dev);
>> +    s = SYS_BUS_DEVICE(dev);
>> +    memory_region_add_subregion(get_system_io(), BIOS_CFG_IOPORT,
>> +                                sysbus_mmio_get_region(s, 0));
> 
> Now that you exported TYPE_FW_CFG_IO I think this might be
> useful/cleaner to have that code in an static inlined function in fw_cfg.h:
> 
> DeviceState *fw_cfg_create(uint32_t iobase[, bool dma_enabled]);
> 
> What do you think?
> 
> Anyway:
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

I'd be inclined to leave the patch in its current form unless anyone
strongly needs more wrapper functions. The important part of the
patchset from my perspective is that it brings the fw_cfg MMIO and
ioport implementations a lot closer together.


ATB,

Mark.