[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/4] exec: fix incorrect assumptions in memory_a
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 4/4] exec: fix incorrect assumptions in memory_access_size |
Date: |
Fri, 19 Jul 2013 22:07:07 -0400 |
On Fri, 19 Jul 2013 22:07:58 +0200
Paolo Bonzini <address@hidden> wrote:
> access_size_min can be 1 because erroneous accesses must not crash
> QEMU, they should trigger exceptions in the guest or just return
> garbage (depending on the CPU). I am not sure I understand the
> comment: placing a 4-byte field at the last byte of a region
> makes no sense (unless impl.unaligned is true), and that is
> why memory.c:access_with_adjusted_size does not bother with
> minimums larger than the remaining length.
>
> access_size_max can be mr->ops->valid.max_access_size because memory.c
> can and will still break accesses bigger than
> mr->ops->impl.max_access_size.
>
> Reported-by: Markus Armbruster <address@hidden>
> Tested-by: Markus Armbruster <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
Yeah, works for me now:
Tested-by: Luiz Capitulino <address@hidden>
> ---
> exec.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index d312bb4..c8658c6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1898,14 +1898,10 @@ static inline bool
> memory_access_is_direct(MemoryRegion *mr, bool is_write)
>
> static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
> {
> - unsigned access_size_min = mr->ops->impl.min_access_size;
> - unsigned access_size_max = mr->ops->impl.max_access_size;
> + unsigned access_size_max = mr->ops->valid.max_access_size;
>
> /* Regions are assumed to support 1-4 byte accesses unless
> otherwise specified. */
> - if (access_size_min == 0) {
> - access_size_min = 1;
> - }
> if (access_size_max == 0) {
> access_size_max = 4;
> }
> @@ -1922,9 +1918,6 @@ static int memory_access_size(MemoryRegion *mr,
> unsigned l, hwaddr addr)
> if (l > access_size_max) {
> l = access_size_max;
> }
> - /* ??? The users of this function are wrong, not supporting minimums
> larger
> - than the remaining length. C.f. memory.c:access_with_adjusted_size.
> */
> - assert(l >= access_size_min);
>
> return l;
> }
- [Qemu-devel] [PULL 0/4] Memory API fixes for soft freeze, Paolo Bonzini, 2013/07/19
- [Qemu-devel] [PATCH 1/4] exec.c: Pass correct pointer type to qemu_ram_ptr_length, Paolo Bonzini, 2013/07/19
- [Qemu-devel] [PATCH 2/4] memory: actually set the owner, Paolo Bonzini, 2013/07/19
- [Qemu-devel] [PATCH 3/4] memory: Return -1 again on reads from unsigned regions, Paolo Bonzini, 2013/07/19
- [Qemu-devel] [PATCH 4/4] exec: fix incorrect assumptions in memory_access_size, Paolo Bonzini, 2013/07/19
- Re: [Qemu-devel] [PATCH 4/4] exec: fix incorrect assumptions in memory_access_size,
Luiz Capitulino <=
- Re: [Qemu-devel] [PULL 0/4] Memory API fixes for soft freeze, Peter Maydell, 2013/07/22