* [PATCH v2 0/2] hw/elf_ops: clear uninitialized segment space
@ 2021-04-15 10:04 Philippe Mathieu-Daudé
2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé
2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
0 siblings, 2 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-15 10:04 UTC (permalink / raw)
To: Stefano Garzarella, Laurent Vivier, qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini
Respining Laurent's patch, but extracting the address_space_set()
helper.
Laurent Vivier (1):
hw/elf_ops: clear uninitialized segment space
Philippe Mathieu-Daudé (1):
exec/memory: Extract address_space_set() from dma_memory_set()
include/exec/memory.h | 16 ++++++++++++++++
include/hw/elf_ops.h | 13 +++++++++++++
softmmu/dma-helpers.c | 16 +---------------
softmmu/physmem.c | 19 +++++++++++++++++++
4 files changed, 49 insertions(+), 15 deletions(-)
--
2.26.3
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()
2021-04-15 10:04 [PATCH v2 0/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
@ 2021-04-15 10:04 ` Philippe Mathieu-Daudé
2021-04-15 10:46 ` Laurent Vivier
` (2 more replies)
2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
1 sibling, 3 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-15 10:04 UTC (permalink / raw)
To: Stefano Garzarella, Laurent Vivier, qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini
dma_memory_set() does a DMA barrier, set the address space with
a constant value. The constant value filling code is not specific
to DMA and can be used for AddressSpace. Extract it as a new
helper: address_space_set().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/exec/memory.h | 16 ++++++++++++++++
softmmu/dma-helpers.c | 16 +---------------
softmmu/physmem.c | 19 +++++++++++++++++++
3 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5728a681b27..192139af58e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2568,6 +2568,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
}
}
+/**
+ * address_space_set: Fill address space with a constant byte.
+ *
+ * Return a MemTxResult indicating whether the operation succeeded
+ * or failed (eg unassigned memory, device rejected the transaction,
+ * IOMMU fault).
+ *
+ * @as: #AddressSpace to be accessed
+ * @addr: address within that address space
+ * @c: constant byte to fill the memory
+ * @len: the number of bytes to fill with the constant byte
+ * @attrs: memory transaction attributes
+ */
+MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
+ uint8_t c, hwaddr len, MemTxAttrs attrs);
+
#ifdef NEED_CPU_H
/* enum device_endian to MemOp. */
static inline MemOp devend_memop(enum device_endian end)
diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
index 7d766a5e89a..8e1e7ad5320 100644
--- a/softmmu/dma-helpers.c
+++ b/softmmu/dma-helpers.c
@@ -23,21 +23,7 @@ MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr,
{
dma_barrier(as, DMA_DIRECTION_FROM_DEVICE);
-#define FILLBUF_SIZE 512
- uint8_t fillbuf[FILLBUF_SIZE];
- int l;
- MemTxResult error = MEMTX_OK;
-
- memset(fillbuf, c, FILLBUF_SIZE);
- while (len > 0) {
- l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
- error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED,
- fillbuf, l);
- len -= l;
- addr += l;
- }
-
- return error;
+ return address_space_set(as, addr, c, len, MEMTXATTRS_UNSPECIFIED);
}
void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 85034d9c11e..c9117527ae7 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2891,6 +2891,25 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
}
}
+MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
+ uint8_t c, hwaddr len, MemTxAttrs attrs)
+{
+#define FILLBUF_SIZE 512
+ uint8_t fillbuf[FILLBUF_SIZE];
+ int l;
+ MemTxResult error = MEMTX_OK;
+
+ memset(fillbuf, c, FILLBUF_SIZE);
+ while (len > 0) {
+ l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
+ error |= address_space_write(as, addr, attrs, fillbuf, l);
+ len -= l;
+ addr += l;
+ }
+
+ return error;
+}
+
void cpu_physical_memory_rw(hwaddr addr, void *buf,
hwaddr len, bool is_write)
{
--
2.26.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space
2021-04-15 10:04 [PATCH v2 0/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé
@ 2021-04-15 10:04 ` Philippe Mathieu-Daudé
2021-04-15 11:02 ` Stefano Garzarella
` (3 more replies)
1 sibling, 4 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-15 10:04 UTC (permalink / raw)
To: Stefano Garzarella, Laurent Vivier, qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini
From: Laurent Vivier <laurent@vivier.eu>
When the mem_size of the segment is bigger than the file_size,
and if this space doesn't overlap another segment, it needs
to be cleared.
This bug is very similar to the one we had for linux-user,
22d113b52f41 ("linux-user: Fix loading of BSS segments"),
where .bss section is encoded as an extension of the the data
one by setting the segment p_memsz > p_filesz.
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20210414105838.205019-1-laurent@vivier.eu>
[PMD: Use recently added address_space_set()]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/hw/elf_ops.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 6ee458e7bc3..29f4c43e231 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -562,6 +562,19 @@ static int glue(load_elf, SZ)(const char *name, int fd,
if (res != MEMTX_OK) {
goto fail;
}
+ /*
+ * We need to zero'ify the space that is not copied
+ * from file
+ */
+ if (file_size < mem_size) {
+ res = address_space_set(as ? as : &address_space_memory,
+ addr + file_size, 0,
+ mem_size - file_size,
+ MEMTXATTRS_UNSPECIFIED);
+ if (res != MEMTX_OK) {
+ goto fail;
+ }
+ }
}
}
--
2.26.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()
2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé
@ 2021-04-15 10:46 ` Laurent Vivier
2021-04-15 11:00 ` Stefano Garzarella
2021-04-15 22:32 ` Richard Henderson
2 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2021-04-15 10:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Stefano Garzarella, qemu-devel
Cc: Peter Maydell, Paolo Bonzini
Le 15/04/2021 à 12:04, Philippe Mathieu-Daudé a écrit :
> dma_memory_set() does a DMA barrier, set the address space with
> a constant value. The constant value filling code is not specific
> to DMA and can be used for AddressSpace. Extract it as a new
> helper: address_space_set().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> include/exec/memory.h | 16 ++++++++++++++++
> softmmu/dma-helpers.c | 16 +---------------
> softmmu/physmem.c | 19 +++++++++++++++++++
> 3 files changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 5728a681b27..192139af58e 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2568,6 +2568,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
> }
> }
>
> +/**
> + * address_space_set: Fill address space with a constant byte.
> + *
> + * Return a MemTxResult indicating whether the operation succeeded
> + * or failed (eg unassigned memory, device rejected the transaction,
> + * IOMMU fault).
> + *
> + * @as: #AddressSpace to be accessed
> + * @addr: address within that address space
> + * @c: constant byte to fill the memory
> + * @len: the number of bytes to fill with the constant byte
> + * @attrs: memory transaction attributes
> + */
> +MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
> + uint8_t c, hwaddr len, MemTxAttrs attrs);
> +
> #ifdef NEED_CPU_H
> /* enum device_endian to MemOp. */
> static inline MemOp devend_memop(enum device_endian end)
> diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
> index 7d766a5e89a..8e1e7ad5320 100644
> --- a/softmmu/dma-helpers.c
> +++ b/softmmu/dma-helpers.c
> @@ -23,21 +23,7 @@ MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr,
> {
> dma_barrier(as, DMA_DIRECTION_FROM_DEVICE);
>
> -#define FILLBUF_SIZE 512
> - uint8_t fillbuf[FILLBUF_SIZE];
> - int l;
> - MemTxResult error = MEMTX_OK;
> -
> - memset(fillbuf, c, FILLBUF_SIZE);
> - while (len > 0) {
> - l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
> - error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED,
> - fillbuf, l);
> - len -= l;
> - addr += l;
> - }
> -
> - return error;
> + return address_space_set(as, addr, c, len, MEMTXATTRS_UNSPECIFIED);
> }
>
> void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 85034d9c11e..c9117527ae7 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2891,6 +2891,25 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
> }
> }
>
> +MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
> + uint8_t c, hwaddr len, MemTxAttrs attrs)
> +{
> +#define FILLBUF_SIZE 512
> + uint8_t fillbuf[FILLBUF_SIZE];
> + int l;
> + MemTxResult error = MEMTX_OK;
> +
> + memset(fillbuf, c, FILLBUF_SIZE);
> + while (len > 0) {
> + l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
> + error |= address_space_write(as, addr, attrs, fillbuf, l);
> + len -= l;
> + addr += l;
> + }
> +
> + return error;
> +}
> +
> void cpu_physical_memory_rw(hwaddr addr, void *buf,
> hwaddr len, bool is_write)
> {
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()
2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé
2021-04-15 10:46 ` Laurent Vivier
@ 2021-04-15 11:00 ` Stefano Garzarella
2021-04-15 11:05 ` Peter Maydell
2021-04-15 22:32 ` Richard Henderson
2 siblings, 1 reply; 12+ messages in thread
From: Stefano Garzarella @ 2021-04-15 11:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Paolo Bonzini, Laurent Vivier, qemu-devel
On Thu, Apr 15, 2021 at 12:04:08PM +0200, Philippe Mathieu-Daudé wrote:
>dma_memory_set() does a DMA barrier, set the address space with
>a constant value. The constant value filling code is not specific
>to DMA and can be used for AddressSpace. Extract it as a new
>helper: address_space_set().
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>---
> include/exec/memory.h | 16 ++++++++++++++++
> softmmu/dma-helpers.c | 16 +---------------
> softmmu/physmem.c | 19 +++++++++++++++++++
> 3 files changed, 36 insertions(+), 15 deletions(-)
>
>diff --git a/include/exec/memory.h b/include/exec/memory.h
>index 5728a681b27..192139af58e 100644
>--- a/include/exec/memory.h
>+++ b/include/exec/memory.h
>@@ -2568,6 +2568,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
> }
> }
>
>+/**
>+ * address_space_set: Fill address space with a constant byte.
>+ *
>+ * Return a MemTxResult indicating whether the operation succeeded
>+ * or failed (eg unassigned memory, device rejected the transaction,
>+ * IOMMU fault).
>+ *
>+ * @as: #AddressSpace to be accessed
>+ * @addr: address within that address space
>+ * @c: constant byte to fill the memory
>+ * @len: the number of bytes to fill with the constant byte
>+ * @attrs: memory transaction attributes
>+ */
>+MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
>+ uint8_t c, hwaddr len, MemTxAttrs attrs);
>+
> #ifdef NEED_CPU_H
> /* enum device_endian to MemOp. */
> static inline MemOp devend_memop(enum device_endian end)
>diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
>index 7d766a5e89a..8e1e7ad5320 100644
>--- a/softmmu/dma-helpers.c
>+++ b/softmmu/dma-helpers.c
>@@ -23,21 +23,7 @@ MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr,
> {
> dma_barrier(as, DMA_DIRECTION_FROM_DEVICE);
>
>-#define FILLBUF_SIZE 512
>- uint8_t fillbuf[FILLBUF_SIZE];
>- int l;
>- MemTxResult error = MEMTX_OK;
>-
>- memset(fillbuf, c, FILLBUF_SIZE);
>- while (len > 0) {
>- l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
>- error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED,
>- fillbuf, l);
>- len -= l;
>- addr += l;
>- }
>-
>- return error;
>+ return address_space_set(as, addr, c, len, MEMTXATTRS_UNSPECIFIED);
> }
>
> void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
>diff --git a/softmmu/physmem.c b/softmmu/physmem.c
>index 85034d9c11e..c9117527ae7 100644
>--- a/softmmu/physmem.c
>+++ b/softmmu/physmem.c
>@@ -2891,6 +2891,25 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
> }
> }
>
>+MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
>+ uint8_t c, hwaddr len, MemTxAttrs attrs)
>+{
>+#define FILLBUF_SIZE 512
>+ uint8_t fillbuf[FILLBUF_SIZE];
>+ int l;
>+ MemTxResult error = MEMTX_OK;
>+
>+ memset(fillbuf, c, FILLBUF_SIZE);
>+ while (len > 0) {
What about return immediately if there is an error?
I mean:
while (len > 0 && result == MEMTX_OK) {
I don't have a strong opinion on that, so in both cases:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>+ l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE;
>+ error |= address_space_write(as, addr, attrs, fillbuf, l);
>+ len -= l;
>+ addr += l;
>+ }
>+
>+ return error;
>+}
>+
> void cpu_physical_memory_rw(hwaddr addr, void *buf,
> hwaddr len, bool is_write)
> {
>--
>2.26.3
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space
2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
@ 2021-04-15 11:02 ` Stefano Garzarella
2021-04-15 13:26 ` Laurent Vivier
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2021-04-15 11:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Paolo Bonzini, Laurent Vivier, qemu-devel
On Thu, Apr 15, 2021 at 12:04:09PM +0200, Philippe Mathieu-Daudé wrote:
>From: Laurent Vivier <laurent@vivier.eu>
>
>When the mem_size of the segment is bigger than the file_size,
>and if this space doesn't overlap another segment, it needs
>to be cleared.
>
>This bug is very similar to the one we had for linux-user,
>22d113b52f41 ("linux-user: Fix loading of BSS segments"),
>where .bss section is encoded as an extension of the the data
>one by setting the segment p_memsz > p_filesz.
>
>Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>Message-Id: <20210414105838.205019-1-laurent@vivier.eu>
>[PMD: Use recently added address_space_set()]
>Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>---
> include/hw/elf_ops.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
>index 6ee458e7bc3..29f4c43e231 100644
>--- a/include/hw/elf_ops.h
>+++ b/include/hw/elf_ops.h
>@@ -562,6 +562,19 @@ static int glue(load_elf, SZ)(const char *name, int fd,
> if (res != MEMTX_OK) {
> goto fail;
> }
>+ /*
>+ * We need to zero'ify the space that is not copied
>+ * from file
>+ */
>+ if (file_size < mem_size) {
>+ res = address_space_set(as ? as : &address_space_memory,
>+ addr + file_size, 0,
>+ mem_size - file_size,
>+ MEMTXATTRS_UNSPECIFIED);
>+ if (res != MEMTX_OK) {
>+ goto fail;
>+ }
>+ }
> }
> }
>
>--
>2.26.3
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()
2021-04-15 11:00 ` Stefano Garzarella
@ 2021-04-15 11:05 ` Peter Maydell
2021-04-15 11:18 ` Stefano Garzarella
0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2021-04-15 11:05 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
Laurent Vivier, QEMU Developers
On Thu, 15 Apr 2021 at 12:00, Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Thu, Apr 15, 2021 at 12:04:08PM +0200, Philippe Mathieu-Daudé wrote:
> >dma_memory_set() does a DMA barrier, set the address space with
> >a constant value. The constant value filling code is not specific
> >to DMA and can be used for AddressSpace. Extract it as a new
> >helper: address_space_set().
> >
> >+MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
> >+ uint8_t c, hwaddr len, MemTxAttrs attrs)
> >+{
> >+#define FILLBUF_SIZE 512
> >+ uint8_t fillbuf[FILLBUF_SIZE];
> >+ int l;
> >+ MemTxResult error = MEMTX_OK;
> >+
> >+ memset(fillbuf, c, FILLBUF_SIZE);
> >+ while (len > 0) {
>
> What about return immediately if there is an error?
> I mean:
> while (len > 0 && result == MEMTX_OK) {
I think that (a) we're just moving code here so we don't want to also
change semantics; (b) there's a comment in memattrs.h that says
* A zero (MEMTX_OK) response means success; anything else is a failure
* of some kind. The memory subsystem will bitwise-OR together results
* if it is synthesizing an operation from multiple smaller accesses.
so in this function "keep going but merge errors" is in keeping with that
principle.
thanks
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()
2021-04-15 11:05 ` Peter Maydell
@ 2021-04-15 11:18 ` Stefano Garzarella
0 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2021-04-15 11:18 UTC (permalink / raw)
To: Peter Maydell
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
Laurent Vivier, QEMU Developers
On Thu, Apr 15, 2021 at 12:05:07PM +0100, Peter Maydell wrote:
>On Thu, 15 Apr 2021 at 12:00, Stefano Garzarella <sgarzare@redhat.com> wrote:
>>
>> On Thu, Apr 15, 2021 at 12:04:08PM +0200, Philippe Mathieu-Daudé wrote:
>> >dma_memory_set() does a DMA barrier, set the address space with
>> >a constant value. The constant value filling code is not specific
>> >to DMA and can be used for AddressSpace. Extract it as a new
>> >helper: address_space_set().
>
>> >
>> >+MemTxResult address_space_set(AddressSpace *as, hwaddr addr,
>> >+ uint8_t c, hwaddr len, MemTxAttrs attrs)
>> >+{
>> >+#define FILLBUF_SIZE 512
>> >+ uint8_t fillbuf[FILLBUF_SIZE];
>> >+ int l;
>> >+ MemTxResult error = MEMTX_OK;
>> >+
>> >+ memset(fillbuf, c, FILLBUF_SIZE);
>> >+ while (len > 0) {
>>
>> What about return immediately if there is an error?
>> I mean:
>> while (len > 0 && result == MEMTX_OK) {
>
>I think that (a) we're just moving code here so we don't want to also
>change semantics; (b) there's a comment in memattrs.h that says
> * A zero (MEMTX_OK) response means success; anything else is a failure
> * of some kind. The memory subsystem will bitwise-OR together results
> * if it is synthesizing an operation from multiple smaller accesses.
>
>so in this function "keep going but merge errors" is in keeping with that
>principle.
Got it, thanks for the explanation! :-)
Stefano
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space
2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
2021-04-15 11:02 ` Stefano Garzarella
@ 2021-04-15 13:26 ` Laurent Vivier
2021-04-15 22:33 ` Richard Henderson
2021-04-17 15:36 ` Laurent Vivier
3 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2021-04-15 13:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Stefano Garzarella, qemu-devel
Cc: Peter Maydell, Paolo Bonzini
Le 15/04/2021 à 12:04, Philippe Mathieu-Daudé a écrit :
> From: Laurent Vivier <laurent@vivier.eu>
>
> When the mem_size of the segment is bigger than the file_size,
> and if this space doesn't overlap another segment, it needs
> to be cleared.
>
> This bug is very similar to the one we had for linux-user,
> 22d113b52f41 ("linux-user: Fix loading of BSS segments"),
> where .bss section is encoded as an extension of the the data
> one by setting the segment p_memsz > p_filesz.
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> Message-Id: <20210414105838.205019-1-laurent@vivier.eu>
> [PMD: Use recently added address_space_set()]
I agree that is a better way to do.
thanks,
Laurent
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> include/hw/elf_ops.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index 6ee458e7bc3..29f4c43e231 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -562,6 +562,19 @@ static int glue(load_elf, SZ)(const char *name, int fd,
> if (res != MEMTX_OK) {
> goto fail;
> }
> + /*
> + * We need to zero'ify the space that is not copied
> + * from file
> + */
> + if (file_size < mem_size) {
> + res = address_space_set(as ? as : &address_space_memory,
> + addr + file_size, 0,
> + mem_size - file_size,
> + MEMTXATTRS_UNSPECIFIED);
> + if (res != MEMTX_OK) {
> + goto fail;
> + }
> + }
> }
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()
2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé
2021-04-15 10:46 ` Laurent Vivier
2021-04-15 11:00 ` Stefano Garzarella
@ 2021-04-15 22:32 ` Richard Henderson
2 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2021-04-15 22:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Stefano Garzarella, Laurent Vivier, qemu-devel
Cc: Peter Maydell, Paolo Bonzini
On 4/15/21 3:04 AM, Philippe Mathieu-Daudé wrote:
> dma_memory_set() does a DMA barrier, set the address space with
> a constant value. The constant value filling code is not specific
> to DMA and can be used for AddressSpace. Extract it as a new
> helper: address_space_set().
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> ---
> include/exec/memory.h | 16 ++++++++++++++++
> softmmu/dma-helpers.c | 16 +---------------
> softmmu/physmem.c | 19 +++++++++++++++++++
> 3 files changed, 36 insertions(+), 15 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space
2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
2021-04-15 11:02 ` Stefano Garzarella
2021-04-15 13:26 ` Laurent Vivier
@ 2021-04-15 22:33 ` Richard Henderson
2021-04-17 15:36 ` Laurent Vivier
3 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2021-04-15 22:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Stefano Garzarella, Laurent Vivier, qemu-devel
Cc: Peter Maydell, Paolo Bonzini
On 4/15/21 3:04 AM, Philippe Mathieu-Daudé wrote:
> From: Laurent Vivier<laurent@vivier.eu>
>
> When the mem_size of the segment is bigger than the file_size,
> and if this space doesn't overlap another segment, it needs
> to be cleared.
>
> This bug is very similar to the one we had for linux-user,
> 22d113b52f41 ("linux-user: Fix loading of BSS segments"),
> where .bss section is encoded as an extension of the the data
> one by setting the segment p_memsz > p_filesz.
>
> Signed-off-by: Laurent Vivier<laurent@vivier.eu>
> Message-Id:<20210414105838.205019-1-laurent@vivier.eu>
> [PMD: Use recently added address_space_set()]
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> ---
> include/hw/elf_ops.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space
2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-04-15 22:33 ` Richard Henderson
@ 2021-04-17 15:36 ` Laurent Vivier
3 siblings, 0 replies; 12+ messages in thread
From: Laurent Vivier @ 2021-04-17 15:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Stefano Garzarella, qemu-devel
Cc: Peter Maydell, Paolo Bonzini
Le 15/04/2021 à 12:04, Philippe Mathieu-Daudé a écrit :
> From: Laurent Vivier <laurent@vivier.eu>
>
> When the mem_size of the segment is bigger than the file_size,
> and if this space doesn't overlap another segment, it needs
> to be cleared.
>
> This bug is very similar to the one we had for linux-user,
> 22d113b52f41 ("linux-user: Fix loading of BSS segments"),
> where .bss section is encoded as an extension of the the data
> one by setting the segment p_memsz > p_filesz.
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> Message-Id: <20210414105838.205019-1-laurent@vivier.eu>
> [PMD: Use recently added address_space_set()]
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> include/hw/elf_ops.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index 6ee458e7bc3..29f4c43e231 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -562,6 +562,19 @@ static int glue(load_elf, SZ)(const char *name, int fd,
> if (res != MEMTX_OK) {
> goto fail;
> }
> + /*
> + * We need to zero'ify the space that is not copied
> + * from file
> + */
> + if (file_size < mem_size) {
> + res = address_space_set(as ? as : &address_space_memory,
> + addr + file_size, 0,
> + mem_size - file_size,
> + MEMTXATTRS_UNSPECIFIED);
> + if (res != MEMTX_OK) {
> + goto fail;
> + }
> + }
> }
> }
>
>
It seems we need also the same kind of operation with the other branch of the if
(rom_add_elf_program()), but I'm not sure where to do it:
diff --git a/hw/core/loader.c b/hw/core/loader.c
index d3e5f3b423f6..8146fdcbb7a0 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1146,9 +1146,13 @@ static void rom_reset(void *unused)
if (rom->mr) {
void *host = memory_region_get_ram_ptr(rom->mr);
memcpy(host, rom->data, rom->datasize);
+ memset(host + rom->datasize, 0, rom->romsize - rom->datasize);
} else {
address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
rom->data, rom->datasize);
+ address_space_set(rom->as, rom->addr + rom->datasize, 0,
+ rom->romsize - rom->datasize,
+ MEMTXATTRS_UNSPECIFIED);
}
if (rom->isrom) {
/* rom needs to be written only once */
Thanks,
Laurent
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-04-17 15:37 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-15 10:04 [PATCH v2 0/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé
2021-04-15 10:46 ` Laurent Vivier
2021-04-15 11:00 ` Stefano Garzarella
2021-04-15 11:05 ` Peter Maydell
2021-04-15 11:18 ` Stefano Garzarella
2021-04-15 22:32 ` Richard Henderson
2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé
2021-04-15 11:02 ` Stefano Garzarella
2021-04-15 13:26 ` Laurent Vivier
2021-04-15 22:33 ` Richard Henderson
2021-04-17 15:36 ` Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).