[Qemu-devel] [PATCH 2/2] Revert "memory: Return -1 again on reads from unsigned regions"

[Qemu-devel] [PATCH 2/2] Revert "memory: Return -1 again on reads from unsigned regions"

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

This reverts commit 9b8c69243585a32d14b9bb9fcd52c37b0b5a1b71.

The commit was wrong: We only return -1 on invalid accesses, not on
valid but unbacked ones. This broke various corner cases.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 memory.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/memory.c b/memory.c
index ac6f3c6..7a0251d 100644
--- a/memory.c
+++ b/memory.c
@@ -873,7 +873,7 @@ static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
     if (current_cpu != NULL) {
         cpu_unassigned_access(current_cpu, addr, false, false, 0, size);
     }
-    return -1ULL;
+    return 0;
 }
 
 static void unassigned_mem_write(void *opaque, hwaddr addr,
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH 2/2] Revert "memory: Return -1 again on reads from unsigned regions"

[Peter Maydell]

On 3 August 2013 09:31, Jan Kiszka <jan.kiszka@web.de> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> This reverts commit 9b8c69243585a32d14b9bb9fcd52c37b0b5a1b71.
>
> The commit was wrong: We only return -1 on invalid accesses, not on
> valid but unbacked ones. This broke various corner cases.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

This revert fixes the regression in the 'musicpal' board.

Presumably these two patches should go into 1.6 since it is
a regression fix... cc'ing Anthony.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH 2/2] Revert "memory: Return -1 again on reads from unsigned regions"

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On 2013-08-08 17:23, Peter Maydell wrote:
> On 3 August 2013 09:31, Jan Kiszka <jan.kiszka@web.de> wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This reverts commit 9b8c69243585a32d14b9bb9fcd52c37b0b5a1b71.
>>
>> The commit was wrong: We only return -1 on invalid accesses, not on
>> valid but unbacked ones. This broke various corner cases.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This revert fixes the regression in the 'musicpal' board.
> 
> Presumably these two patches should go into 1.6 since it is
> a regression fix... cc'ing Anthony.
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Yes, for 1.6 definitely. Sorry, forgot the proper CCing/tagging.

Thanks,
Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [Qemu-devel] [PATCH 2/2] Revert "memory: Return -1 again on reads from unsigned regions"

[Andreas Färber]

Am 08.08.2013 17:23, schrieb Peter Maydell:
> On 3 August 2013 09:31, Jan Kiszka <jan.kiszka@web.de> wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This reverts commit 9b8c69243585a32d14b9bb9fcd52c37b0b5a1b71.
>>
>> The commit was wrong: We only return -1 on invalid accesses, not on
>> valid but unbacked ones. This broke various corner cases.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This revert fixes the regression in the 'musicpal' board.
> 
> Presumably these two patches should go into 1.6 since it is
> a regression fix... cc'ing Anthony.

Since no one has spoken up with a concrete use case that breaks when
having the PIO region opaque, I agree this seems the best solution we
have for 1.6. But given the musicpal issue you have raised, I would ask
Paolo or Jan to squash these two patches together to avoid a git-bisect
regression.

Thanks,
Andreas

> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> -- PMM
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/2] Revert "memory: Return -1 again on reads from unsigned regions"

[Peter Maydell]

On 8 August 2013 17:46, Andreas Färber <afaerber@suse.de> wrote:
> Since no one has spoken up with a concrete use case that breaks when
> having the PIO region opaque, I agree this seems the best solution we
> have for 1.6. But given the musicpal issue you have raised, I would ask
> Paolo or Jan to squash these two patches together to avoid a git-bisect
> regression.

Not sure what you have in mind here -- musicpal is an ARM
board, and no ARM system should ever touch the system_io
region at all, so for musicpal patch 1/2 has no effect
and we care only about 2/2.

-- PMM

Re: [Qemu-devel] [PATCH 2/2] Revert "memory: Return -1 again on reads from unsigned regions"

[Andreas Färber]

Am 08.08.2013 18:48, schrieb Peter Maydell:
> On 8 August 2013 17:46, Andreas Färber <afaerber@suse.de> wrote:
>> Since no one has spoken up with a concrete use case that breaks when
>> having the PIO region opaque, I agree this seems the best solution we
>> have for 1.6. But given the musicpal issue you have raised, I would ask
>> Paolo or Jan to squash these two patches together to avoid a git-bisect
>> regression.
> 
> Not sure what you have in mind here -- musicpal is an ARM
> board, and no ARM system should ever touch the system_io
> region at all, so for musicpal patch 1/2 has no effect
> and we care only about 2/2.

Sorry, mixed up your other reply with this non-threaded series - thought
you were saying 1/2 introduced the regression, but the regression is
already in qemu.git I understand just like for prep.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg