diff mbox

[02/13] Implement cpu_physical_memory_zero()

Message ID 1336625347-10169-3-git-send-email-benh@kernel.crashing.org
State New
Headers show

Commit Message

Benjamin Herrenschmidt May 10, 2012, 4:48 a.m. UTC
From: David Gibson <david@gibson.dropbear.id.au>

This patch adds cpu_physical_memory_zero() function.  This is equivalent to
calling cpu_physical_memory_write() with a buffer full of zeroes, but
avoids actually allocating such a buffer along the way.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 cpu-common.h |    1 +
 exec.c       |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

Comments

Anthony Liguori May 15, 2012, 12:42 a.m. UTC | #1
On 05/09/2012 11:48 PM, Benjamin Herrenschmidt wrote:
> From: David Gibson<david@gibson.dropbear.id.au>
>
> This patch adds cpu_physical_memory_zero() function.  This is equivalent to
> calling cpu_physical_memory_write() with a buffer full of zeroes, but
> avoids actually allocating such a buffer along the way.
>
> Signed-off-by: David Gibson<david@gibson.dropbear.id.au>
> Signed-off-by: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> ---
>   cpu-common.h |    1 +
>   exec.c       |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 54 insertions(+)
>
> diff --git a/cpu-common.h b/cpu-common.h
> index dca5175..146429c 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -53,6 +53,7 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev);
>
>   void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>                               int len, int is_write);
> +void cpu_physical_memory_zero(target_phys_addr_t addr, int len);
>   static inline void cpu_physical_memory_read(target_phys_addr_t addr,
>                                               void *buf, int len)
>   {
> diff --git a/exec.c b/exec.c
> index 0607c9b..8511496 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3581,6 +3581,59 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>       }
>   }
>
> +void cpu_physical_memory_zero(target_phys_addr_t addr, int len)
> +{

I'd think a memset() like interface would be better but...

We should definitely implement this function in terms of 
cpu_physical_memory_write instead of open coding the logic again.

Regards,

Anthony Liguori

> +    int l;
> +    uint8_t *ptr;
> +    target_phys_addr_t page;
> +    MemoryRegionSection *section;
> +
> +    while (len>  0) {
> +        page = addr&  TARGET_PAGE_MASK;
> +        l = (page + TARGET_PAGE_SIZE) - addr;
> +        if (l>  len)
> +            l = len;
> +        section = phys_page_find(page>>  TARGET_PAGE_BITS);
> +
> +        if (!memory_region_is_ram(section->mr)) {
> +            target_phys_addr_t addr1;
> +            addr1 = memory_region_section_addr(section, addr);
> +            /* XXX: could force cpu_single_env to NULL to avoid
> +               potential bugs */
> +            if (l>= 4&&  ((addr1&  3) == 0)) {
> +                /* 32 bit write access */
> +                io_mem_write(section->mr, addr1, 0, 4);
> +                l = 4;
> +            } else if (l>= 2&&  ((addr1&  1) == 0)) {
> +                /* 16 bit write access */
> +                io_mem_write(section->mr, addr1, 0, 2);
> +                l = 2;
> +            } else {
> +                /* 8 bit write access */
> +                io_mem_write(section->mr, addr1, 0, 1);
> +                l = 1;
> +            }
> +        } else if (!section->readonly) {
> +            ram_addr_t addr1;
> +            addr1 = memory_region_get_ram_addr(section->mr)
> +                + memory_region_section_addr(section, addr);
> +            /* RAM case */
> +            ptr = qemu_get_ram_ptr(addr1);
> +            memset(ptr, 0, l);
> +            if (!cpu_physical_memory_is_dirty(addr1)) {
> +                /* invalidate code */
> +                tb_invalidate_phys_page_range(addr1, addr1 + l, 0);
> +                /* set dirty bit */
> +                cpu_physical_memory_set_dirty_flags(
> +                    addr1, (0xff&  ~CODE_DIRTY_FLAG));
> +            }
> +            qemu_put_ram_ptr(ptr);
> +        }
> +        len -= l;
> +        addr += l;
> +    }
> +}
> +
>   /* used for ROM loading : can write in RAM and ROM */
>   void cpu_physical_memory_write_rom(target_phys_addr_t addr,
>                                      const uint8_t *buf, int len)
David Gibson May 15, 2012, 1:23 a.m. UTC | #2
On Mon, May 14, 2012 at 07:42:00PM -0500, Anthony Liguori wrote:
> On 05/09/2012 11:48 PM, Benjamin Herrenschmidt wrote:
> >From: David Gibson<david@gibson.dropbear.id.au>
[snip]
> >@@ -3581,6 +3581,59 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
> >      }
> >  }
> >
> >+void cpu_physical_memory_zero(target_phys_addr_t addr, int len)
> >+{
> 
> I'd think a memset() like interface would be better but...

I can work with that.

> We should definitely implement this function in terms of
> cpu_physical_memory_write instead of open coding the logic again.

Hrm.  Having solved merge conflicts several times by recopying the
cpu_physical_memory_rw() logic, I can certainly see the attraction in
that.  However, the point of this function is *not* to have to
allocate a temporary buffer, and I don't really see how to combine the
logic without that.
Anthony Liguori May 15, 2012, 2:03 a.m. UTC | #3
On 05/14/2012 08:23 PM, David Gibson wrote:
> On Mon, May 14, 2012 at 07:42:00PM -0500, Anthony Liguori wrote:
>> On 05/09/2012 11:48 PM, Benjamin Herrenschmidt wrote:
>>> From: David Gibson<david@gibson.dropbear.id.au>
> [snip]
>>> @@ -3581,6 +3581,59 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
>>>       }
>>>   }
>>>
>>> +void cpu_physical_memory_zero(target_phys_addr_t addr, int len)
>>> +{
>>
>> I'd think a memset() like interface would be better but...
>
> I can work with that.
>
>> We should definitely implement this function in terms of
>> cpu_physical_memory_write instead of open coding the logic again.
>
> Hrm.  Having solved merge conflicts several times by recopying the
> cpu_physical_memory_rw() logic, I can certainly see the attraction in
> that.  However, the point of this function is *not* to have to
> allocate a temporary buffer, and I don't really see how to combine the
> logic without that.

Just use a fixed buffer (uint8_t buffer[512]) and call cpu_physical_memory_rw 
multiple times with an offset.

Regards,

Anthony Liguori

>
diff mbox

Patch

diff --git a/cpu-common.h b/cpu-common.h
index dca5175..146429c 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -53,6 +53,7 @@  void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev);
 
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write);
+void cpu_physical_memory_zero(target_phys_addr_t addr, int len);
 static inline void cpu_physical_memory_read(target_phys_addr_t addr,
                                             void *buf, int len)
 {
diff --git a/exec.c b/exec.c
index 0607c9b..8511496 100644
--- a/exec.c
+++ b/exec.c
@@ -3581,6 +3581,59 @@  void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
     }
 }
 
+void cpu_physical_memory_zero(target_phys_addr_t addr, int len)
+{
+    int l;
+    uint8_t *ptr;
+    target_phys_addr_t page;
+    MemoryRegionSection *section;
+
+    while (len > 0) {
+        page = addr & TARGET_PAGE_MASK;
+        l = (page + TARGET_PAGE_SIZE) - addr;
+        if (l > len)
+            l = len;
+        section = phys_page_find(page >> TARGET_PAGE_BITS);
+
+        if (!memory_region_is_ram(section->mr)) {
+            target_phys_addr_t addr1;
+            addr1 = memory_region_section_addr(section, addr);
+            /* XXX: could force cpu_single_env to NULL to avoid
+               potential bugs */
+            if (l >= 4 && ((addr1 & 3) == 0)) {
+                /* 32 bit write access */
+                io_mem_write(section->mr, addr1, 0, 4);
+                l = 4;
+            } else if (l >= 2 && ((addr1 & 1) == 0)) {
+                /* 16 bit write access */
+                io_mem_write(section->mr, addr1, 0, 2);
+                l = 2;
+            } else {
+                /* 8 bit write access */
+                io_mem_write(section->mr, addr1, 0, 1);
+                l = 1;
+            }
+        } else if (!section->readonly) {
+            ram_addr_t addr1;
+            addr1 = memory_region_get_ram_addr(section->mr)
+                + memory_region_section_addr(section, addr);
+            /* RAM case */
+            ptr = qemu_get_ram_ptr(addr1);
+            memset(ptr, 0, l);
+            if (!cpu_physical_memory_is_dirty(addr1)) {
+                /* invalidate code */
+                tb_invalidate_phys_page_range(addr1, addr1 + l, 0);
+                /* set dirty bit */
+                cpu_physical_memory_set_dirty_flags(
+                    addr1, (0xff & ~CODE_DIRTY_FLAG));
+            }
+            qemu_put_ram_ptr(ptr);
+        }
+        len -= l;
+        addr += l;
+    }
+}
+
 /* used for ROM loading : can write in RAM and ROM */
 void cpu_physical_memory_write_rom(target_phys_addr_t addr,
                                    const uint8_t *buf, int len)