[Qemu-devel] [PATCH] exec: move io_mem_read/write to memory.h

[Qemu-devel] [PATCH] exec: move io_mem_read/write to memory.h

[Michael S. Tsirkin]

implementation is in memory.c, move function
to match. This allows use from places that
don't pull in exec-all.h

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

 include/exec/exec-all.h | 5 -----
 include/exec/memory.h   | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6362074..28cb37d 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -367,11 +367,6 @@ bool is_tcg_gen_code(uintptr_t pc_ptr);
 #if !defined(CONFIG_USER_ONLY)
 
 struct MemoryRegion *iotlb_to_region(hwaddr index);
-uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
-                     unsigned size);
-void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
-                  uint64_t value, unsigned size);
-
 void tlb_fill(CPUArchState *env1, target_ulong addr, int is_write, int mmu_idx,
               uintptr_t retaddr);
 
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9e88320..edeb1f2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -888,6 +888,11 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
                          int is_write, hwaddr access_len);
 
 
+uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
+                     unsigned size);
+void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
+                  uint64_t value, unsigned size);
+
 #endif
 
 #endif
-- 
MST

Re: [Qemu-devel] [PATCH] exec: move io_mem_read/write to memory.h

[Paolo Bonzini]

Il 13/06/2013 08:59, Michael S. Tsirkin ha scritto:
> implementation is in memory.c, move function
> to match. This allows use from places that
> don't pull in exec-all.h

But they shouldn't be used. :)

Everything except the current users (TCG, and address_space_rw and
friends) should go through exec.c.

Paolo

> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
>  include/exec/exec-all.h | 5 -----
>  include/exec/memory.h   | 5 +++++
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 6362074..28cb37d 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -367,11 +367,6 @@ bool is_tcg_gen_code(uintptr_t pc_ptr);
>  #if !defined(CONFIG_USER_ONLY)
>  
>  struct MemoryRegion *iotlb_to_region(hwaddr index);
> -uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
> -                     unsigned size);
> -void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
> -                  uint64_t value, unsigned size);
> -
>  void tlb_fill(CPUArchState *env1, target_ulong addr, int is_write, int mmu_idx,
>                uintptr_t retaddr);
>  
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 9e88320..edeb1f2 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -888,6 +888,11 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
>                           int is_write, hwaddr access_len);
>  
>  
> +uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
> +                     unsigned size);
> +void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
> +                  uint64_t value, unsigned size);
> +
>  #endif
>  
>  #endif
> 

Re: [Qemu-devel] [PATCH] exec: move io_mem_read/write to memory.h

[Michael S. Tsirkin]

On Thu, Jun 13, 2013 at 06:04:41PM -0400, Paolo Bonzini wrote:
> Il 13/06/2013 08:59, Michael S. Tsirkin ha scritto:
> > implementation is in memory.c, move function
> > to match. This allows use from places that
> > don't pull in exec-all.h
> 
> But they shouldn't be used. :)
> 
> Everything except the current users (TCG, and address_space_rw and
> friends) should go through exec.c.
> 
> Paolo

OK but still. It's an interface that memory.c exports.

If you want to mark it as internal, make this clear
in the name but IMO it's a bad idea to force
everyone to use grep to find where the implementation
of each function is.

> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > 
> >  include/exec/exec-all.h | 5 -----
> >  include/exec/memory.h   | 5 +++++
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> > index 6362074..28cb37d 100644
> > --- a/include/exec/exec-all.h
> > +++ b/include/exec/exec-all.h
> > @@ -367,11 +367,6 @@ bool is_tcg_gen_code(uintptr_t pc_ptr);
> >  #if !defined(CONFIG_USER_ONLY)
> >  
> >  struct MemoryRegion *iotlb_to_region(hwaddr index);
> > -uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
> > -                     unsigned size);
> > -void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
> > -                  uint64_t value, unsigned size);
> > -
> >  void tlb_fill(CPUArchState *env1, target_ulong addr, int is_write, int mmu_idx,
> >                uintptr_t retaddr);
> >  
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index 9e88320..edeb1f2 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -888,6 +888,11 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
> >                           int is_write, hwaddr access_len);
> >  
> >  
> > +uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
> > +                     unsigned size);
> > +void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
> > +                  uint64_t value, unsigned size);
> > +
> >  #endif
> >  
> >  #endif
> > 

Re: [Qemu-devel] [PATCH] exec: move io_mem_read/write to memory.h

[Paolo Bonzini]

Il 13/06/2013 18:26, Michael S. Tsirkin ha scritto:
> On Thu, Jun 13, 2013 at 06:04:41PM -0400, Paolo Bonzini wrote:
>> Il 13/06/2013 08:59, Michael S. Tsirkin ha scritto:
>>> implementation is in memory.c, move function
>>> to match. This allows use from places that
>>> don't pull in exec-all.h
>>
>> But they shouldn't be used. :)
>>
>> Everything except the current users (TCG, and address_space_rw and
>> friends) should go through exec.c.
>>
>> Paolo
> 
> OK but still. It's an interface that memory.c exports.
> 
> If you want to mark it as internal, make this clear
> in the name but IMO it's a bad idea to force
> everyone to use grep to find where the implementation
> of each function is.

Yes, I agree.   To add to the mess, io_mem_{read,write} are just
old-fashioned names for (static) memory_region_dispatch_{read,write}.

I need to understand whether/how the per-arch calls to
softmmu_template.h could be moved to exec.c.  Then io_mem_{read,write}
can just disappear, and memory_region_dispatch_{read,write} can move to
memory-internal.h.

Paolo

>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>
>>>  include/exec/exec-all.h | 5 -----
>>>  include/exec/memory.h   | 5 +++++
>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
>>> index 6362074..28cb37d 100644
>>> --- a/include/exec/exec-all.h
>>> +++ b/include/exec/exec-all.h
>>> @@ -367,11 +367,6 @@ bool is_tcg_gen_code(uintptr_t pc_ptr);
>>>  #if !defined(CONFIG_USER_ONLY)
>>>  
>>>  struct MemoryRegion *iotlb_to_region(hwaddr index);
>>> -uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
>>> -                     unsigned size);
>>> -void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
>>> -                  uint64_t value, unsigned size);
>>> -
>>>  void tlb_fill(CPUArchState *env1, target_ulong addr, int is_write, int mmu_idx,
>>>                uintptr_t retaddr);
>>>  
>>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>>> index 9e88320..edeb1f2 100644
>>> --- a/include/exec/memory.h
>>> +++ b/include/exec/memory.h
>>> @@ -888,6 +888,11 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
>>>                           int is_write, hwaddr access_len);
>>>  
>>>  
>>> +uint64_t io_mem_read(struct MemoryRegion *mr, hwaddr addr,
>>> +                     unsigned size);
>>> +void io_mem_write(struct MemoryRegion *mr, hwaddr addr,
>>> +                  uint64_t value, unsigned size);
>>> +
>>>  #endif
>>>  
>>>  #endif
>>>
> 
>