Patchwork [12/12] hw: move memory API to hw/core

login
register
mail settings
Submitter Paolo Bonzini
Date April 11, 2013, 4:10 p.m.
Message ID <1365696633-30727-13-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/235836/
State New
Headers show

Comments

Paolo Bonzini - April 11, 2013, 4:10 p.m.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.target              | 4 ++--
 hw/core/Makefile.objs        | 3 +++
 ioport.c => hw/core/ioport.c | 0
 memory.c => hw/core/memory.c | 0
 4 files changed, 5 insertions(+), 2 deletions(-)
 rename ioport.c => hw/core/ioport.c (100%)
 rename memory.c => hw/core/memory.c (100%)

diff --git a/ioport.c b/hw/core/ioport.c
similarity index 100%
rename from ioport.c
rename to hw/core/ioport.c
diff --git a/memory.c b/hw/core/memory.c
similarity index 100%
rename from memory.c
rename to hw/core/memory.c
Michael S. Tsirkin - April 11, 2013, 5:07 p.m.
On Thu, Apr 11, 2013 at 05:28:35PM +0100, Peter Maydell wrote:
> On 11 April 2013 17:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> This doesn't look right. The MemoryRegion system isn't
> hw-specific, it's a part of the basic QEMU emulation
> system which provides functionality to hw/ and other
> things (like cputlb.c).
> 
> -- PMM

I had the same thought.
Paolo Bonzini - April 11, 2013, 5:09 p.m.
Il 11/04/2013 18:28, Peter Maydell ha scritto:
> On 11 April 2013 17:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> This doesn't look right. The MemoryRegion system isn't
> hw-specific, it's a part of the basic QEMU emulation
> system which provides functionality to hw/ and other
> things (like cputlb.c).

The accelerator- and target-independent parts of the basic emulation are
already in hw/core (not much really, but consider that CPUs are device
and depend on hw/core/qdev.c).  The memory API is simply the interface
between the accelerators and hw/ (boards & device models).

In fact, apart from large parts of exec.c which are really part of the
memory API implementation (but I'm not moving it in this patch for
simplicity), the other users of the memory API are really the
accelerators.  KVM and Xen only use listeners, TCG needs more low-level
access in cputlb.c and translate-all.c but that's pretty much it.

Paolo
Peter Maydell - April 11, 2013, 5:14 p.m.
On 11 April 2013 18:09, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 11/04/2013 18:28, Peter Maydell ha scritto:
>> On 11 April 2013 17:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> This doesn't look right. The MemoryRegion system isn't
>> hw-specific, it's a part of the basic QEMU emulation
>> system which provides functionality to hw/ and other
>> things (like cputlb.c).
>
> The accelerator- and target-independent parts of the basic emulation are
> already in hw/core (not much really, but consider that CPUs are device
> and depend on hw/core/qdev.c).  The memory API is simply the interface
> between the accelerators and hw/ (boards & device models).

Yes, so it should be provided in the place we put our accelerator
implementation (ie .): it is functionality and interface exposed
*to* the code in hw/, not functionality and interface provided
*by* hw/.

-- PMM
Paolo Bonzini - April 11, 2013, 6:18 p.m.
Il 11/04/2013 19:14, Peter Maydell ha scritto:
> On 11 April 2013 18:09, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 11/04/2013 18:28, Peter Maydell ha scritto:
>>> On 11 April 2013 17:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> This doesn't look right. The MemoryRegion system isn't
>>> hw-specific, it's a part of the basic QEMU emulation
>>> system which provides functionality to hw/ and other
>>> things (like cputlb.c).
>>
>> The accelerator- and target-independent parts of the basic emulation are
>> already in hw/core (not much really, but consider that CPUs are device
>> and depend on hw/core/qdev.c).  The memory API is simply the interface
>> between the accelerators and hw/ (boards & device models).
> 
> Yes, so it should be provided in the place we put our accelerator
> implementation (ie .): it is functionality and interface exposed
> *to* the code in hw/, not functionality and interface provided
> *by* hw/.

Ok, the historical practice was that qdev core was in hw/, and that's
what I tried to follow.  It makes sense either way to me.

But then patch 11 also has to be dropped, otherwise it doesn't make
sense.  Michael, what do you think?

Paolo
Michael S. Tsirkin - April 11, 2013, 7 p.m.
On Thu, Apr 11, 2013 at 08:18:52PM +0200, Paolo Bonzini wrote:
> Il 11/04/2013 19:14, Peter Maydell ha scritto:
> > On 11 April 2013 18:09, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> Il 11/04/2013 18:28, Peter Maydell ha scritto:
> >>> On 11 April 2013 17:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >>> This doesn't look right. The MemoryRegion system isn't
> >>> hw-specific, it's a part of the basic QEMU emulation
> >>> system which provides functionality to hw/ and other
> >>> things (like cputlb.c).
> >>
> >> The accelerator- and target-independent parts of the basic emulation are
> >> already in hw/core (not much really, but consider that CPUs are device
> >> and depend on hw/core/qdev.c).  The memory API is simply the interface
> >> between the accelerators and hw/ (boards & device models).
> > 
> > Yes, so it should be provided in the place we put our accelerator
> > implementation (ie .): it is functionality and interface exposed
> > *to* the code in hw/, not functionality and interface provided
> > *by* hw/.
> 
> Ok, the historical practice was that qdev core was in hw/, and that's
> what I tried to follow.  It makes sense either way to me.
> 
> But then patch 11 also has to be dropped, otherwise it doesn't make
> sense.  Michael, what do you think?
> 
> Paolo

I agree.

Patch

diff --git a/Makefile.target b/Makefile.target
index 2bd6d14..9ddb9fa 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -108,13 +108,13 @@  CONFIG_NO_XEN = $(if $(subst n,,$(CONFIG_XEN)),n,y)
 CONFIG_NO_GET_MEMORY_MAPPING = $(if $(subst n,,$(CONFIG_HAVE_GET_MEMORY_MAPPING)),n,y)
 CONFIG_NO_CORE_DUMP = $(if $(subst n,,$(CONFIG_HAVE_CORE_DUMP)),n,y)
 
-obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o
+obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o
 obj-y += qtest.o
 obj-y += hw/
 obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
-obj-y += memory.o savevm.o cputlb.o
+obj-y += savevm.o cputlb.o
 obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
 obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
 obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 94109f3..b9e96ca 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -11,4 +11,7 @@  common-obj-$(CONFIG_SOFTMMU) += null-machine.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-addr.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
+common-obj-$(CONFIG_SOFTMMU) += ioport.o
+obj-$(CONFIG_SOFTMMU) += memory.o
+