Re: [Qemu-devel] [PATCH v1 2/3] memory: Add sysbus memory device

Re: [Qemu-devel] [PATCH v1 2/3] memory: Add sysbus memory device

[Andreas Färber]

Peter,

Am 15.04.2014 04:21, schrieb Peter Crosthwaite:
> Add a sysbus device consisting of a single ram. This allows for
> instantiation of RAM just like any other device. There are a number
> of good reasons to want to do this this:
> 
> 1: Consistency. RAM is not that special where board level files should
> have to instantiate it with a completely different API. This reduces
> complexity of board level development by hiding the memory API
> completely and handling everything via the sysbus API.
> 
> 2: Device tree completeness. Ram Now shows up in info-qtree and
> friends. E.g. Info qtree gives meaningful information under the
> root system bus:
> 
>   dev: sysbus-memory, id "zynq.ocm_ram"
>     size = 262144 (0x40000)
>     read-only = false
>     irq 0
>     mmio 00000000fffc0000/0000000000040000
>   dev: sysbus-memory, id "zynq.ext_ram"
>     size = 134217728 (0x8000000)
>     read-only = false
>     irq 0
>     mmio 0000000000000000/0000000008000000

I had warned that I would nack any patch that justifies changes with
"info qtree", so here's your gentle NAK.

Please help deciding on the following patches, which do show such
devices the QOM way:

http://patchwork.ozlabs.org/patch/317224/
http://patchwork.ozlabs.org/patch/343136/
http://patchwork.ozlabs.org/patch/347064/

Note that this doesn't mean we can't create new SysBusDevices, it means
the commit message should be changed.

Regards,
Andreas

> 
> 3: Remove dependence of global state. Board files don't have to
> explicity request the global singleton (and much unloved)
> address_space_memory() and go hacking on it. address_space_memory()
> is still ultimately used, but the ugliness is hidden in one place - the
> sysbus core (we can fix that another day).
> 
> 4: Data driven machine creation. There is list discussion on being able
> to create or append-to sysbus machines in a data-driven way (whether
> thats from command-line, monitor or scripts or whatever). This patch
> removes the memory special case from that problem and allows RAM
> instantiation to come via whatever solutions we come up with sysbus
> device instantiation.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> 
>  hw/core/Makefile.objs   |  1 +
>  hw/core/sysbus-memory.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+)
>  create mode 100644 hw/core/sysbus-memory.c

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v1 2/3] memory: Add sysbus memory device

[Peter Crosthwaite]

On Fri, May 16, 2014 at 3:22 PM, Andreas Färber <afaerber@suse.de> wrote:
> Peter,
>
> Am 15.04.2014 04:21, schrieb Peter Crosthwaite:
>> Add a sysbus device consisting of a single ram. This allows for
>> instantiation of RAM just like any other device. There are a number
>> of good reasons to want to do this this:
>>
>> 1: Consistency. RAM is not that special where board level files should
>> have to instantiate it with a completely different API. This reduces
>> complexity of board level development by hiding the memory API
>> completely and handling everything via the sysbus API.
>>
>> 2: Device tree completeness. Ram Now shows up in info-qtree and
>> friends. E.g. Info qtree gives meaningful information under the
>> root system bus:
>>
>>   dev: sysbus-memory, id "zynq.ocm_ram"
>>     size = 262144 (0x40000)
>>     read-only = false
>>     irq 0
>>     mmio 00000000fffc0000/0000000000040000
>>   dev: sysbus-memory, id "zynq.ext_ram"
>>     size = 134217728 (0x8000000)
>>     read-only = false
>>     irq 0
>>     mmio 0000000000000000/0000000008000000
>
> I had warned that I would nack any patch that justifies changes with
> "info qtree", so here's your gentle NAK.
>
> Please help deciding on the following patches, which do show such
> devices the QOM way:
>

So I'm shelving this for the interim anyway. My latests series may
infact obsolete this with full MR qomifcation. You could prehaps just
setup TYPE_MEMORY_REGION_RAM (.parent = TYPE_MEMORY_REGION). Then
object-new and friends can be used to achieve the same set of goals as
stated in this commit message (except #3).

> http://patchwork.ozlabs.org/patch/317224/
> http://patchwork.ozlabs.org/patch/343136/
> http://patchwork.ozlabs.org/patch/347064/
>

Yep, this latest one is already on my "to-review" list.

Regards,
Peter

> Note that this doesn't mean we can't create new SysBusDevices, it means
> the commit message should be changed.
>


> Regards,
> Andreas
>
>>
>> 3: Remove dependence of global state. Board files don't have to
>> explicity request the global singleton (and much unloved)
>> address_space_memory() and go hacking on it. address_space_memory()
>> is still ultimately used, but the ugliness is hidden in one place - the
>> sysbus core (we can fix that another day).
>>
>> 4: Data driven machine creation. There is list discussion on being able
>> to create or append-to sysbus machines in a data-driven way (whether
>> thats from command-line, monitor or scripts or whatever). This patch
>> removes the memory special case from that problem and allows RAM
>> instantiation to come via whatever solutions we come up with sysbus
>> device instantiation.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>
>>  hw/core/Makefile.objs   |  1 +
>>  hw/core/sysbus-memory.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 64 insertions(+)
>>  create mode 100644 hw/core/sysbus-memory.c
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

Re: [Qemu-devel] [PATCH v1 2/3] memory: Add sysbus memory device

[Peter Crosthwaite]

On Wed, Apr 16, 2014 at 10:24 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 15.04.14 04:21, Peter Crosthwaite wrote:
>>
>> Add a sysbus device consisting of a single ram. This allows for
>> instantiation of RAM just like any other device. There are a number
>> of good reasons to want to do this this:
>>
>> 1: Consistency. RAM is not that special where board level files should
>> have to instantiate it with a completely different API. This reduces
>> complexity of board level development by hiding the memory API
>> completely and handling everything via the sysbus API.
>>
>> 2: Device tree completeness. Ram Now shows up in info-qtree and
>> friends. E.g. Info qtree gives meaningful information under the
>> root system bus:
>>
>>    dev: sysbus-memory, id "zynq.ocm_ram"
>>      size = 262144 (0x40000)
>>      read-only = false
>>      irq 0
>>      mmio 00000000fffc0000/0000000000040000
>>    dev: sysbus-memory, id "zynq.ext_ram"
>>      size = 134217728 (0x8000000)
>>      read-only = false
>>      irq 0
>>      mmio 0000000000000000/0000000008000000
>>
>> 3: Remove dependence of global state. Board files don't have to
>> explicity request the global singleton (and much unloved)
>> address_space_memory() and go hacking on it. address_space_memory()
>> is still ultimately used, but the ugliness is hidden in one place - the
>> sysbus core (we can fix that another day).
>>
>> 4: Data driven machine creation. There is list discussion on being able
>> to create or append-to sysbus machines in a data-driven way (whether
>> thats from command-line, monitor or scripts or whatever). This patch
>> removes the memory special case from that problem and allows RAM
>> instantiation to come via whatever solutions we come up with sysbus
>> device instantiation.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
>
> Could you please show that this approach works for more complicated
> machines, like x86's pc machine and its PCI holes?
>

Hi Alex,

Do you mean attaching it within a PCI address space? Im not sure
that's valid. That would require some sort of generalisation that
applied to both sysbus and PCI. I actually had a go at something like
this a while back. Basically, the setup was for PCI devices to inherit
from sysbus. This allowed solving the reverse problem. Attaching a PCI
device to sysbus. I guess with a few changes it could be made to work
both ways, but it seems the sysbus and PC world are completely
separate the way the tree stands today.

Regards,
Peter

>
> Alex
>
>