Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()

From: Peter Maydell <peter.maydell@linaro.org>
To: Stefano Garzarella <sgarzare@redhat.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"QEMU Developers" <qemu-devel@nongnu.org>
Subject: Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()
Date: Thu, 15 Apr 2021 12:05:07 +0100	[thread overview]
Message-ID: <CAFEAcA9=hGVPgN0bCcgkRmqGG-ouHwz09VxXHBH-eyToWugRPA@mail.gmail.com> (raw)
In-Reply-To: <20210415110047.ku3uqj5evcprs77r@steredhat>

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