[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_s
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw |
Date: |
Wed, 17 Jul 2013 13:28:11 -0500 |
User-agent: |
Notmuch/0.15.2+202~g0c4b8aa (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Paolo Bonzini <address@hidden> writes:
> Il 17/07/2013 17:50, Anthony Liguori ha scritto:
>> Paolo Bonzini <address@hidden> writes:
>>
>>> Il 17/07/2013 11:50, Markus Armbruster ha scritto:
>>>> Richard Henderson <address@hidden> writes:
>>>>
>>>>> Honor the implementation maximum access size, and at least check
>>>>> the minimum access size.
>>>>>
>>>>> Reviewed-by: Paolo Bonzini <address@hidden>
>>>>> Signed-off-by: Richard Henderson <address@hidden>
>>>>
>>>> Fails for me:
>>>>
>>>> qemu-system-x86_64: /work/armbru/qemu/exec.c:1927: memory_access_size:
>>>> Assertion `l >= access_size_min' failed.
>>>
>>> This:
>>>
>>> unsigned access_size_min = mr->ops->impl.min_access_size;
>>> unsigned access_size_max = mr->ops->impl.max_access_size;
>>>
>>> must be respectively:
>>>
>>> unsigned access_size_min = 1;
>>> unsigned access_size_max = mr->ops->valid.max_access_size;
>>>
>>> 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'm 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).
>>>
>>> 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.
>>>
>>> Markus, can you try the minimal patch above? Or this one that also
>>> does the consequent simplifications.
>>
>> FYI, the reproducer is very simple:
>>
>> qemu-system-x86_64 -usb
>
> My patch works.
Yes, can you send a SoB and submit as a top level?
Right now uhci is completely broken.
Regards,
Anthony Liguori
>
> Paolo
>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>> diff --git a/exec.c b/exec.c
>>> index c99a883..0904283 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -1898,14 +1898,8 @@ 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 +1916,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;
>>> }
>>>
>>> Paolo
>>
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw, (continued)
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw, Anthony Liguori, 2013/07/17
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw, Richard Henderson, 2013/07/17
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw, Richard Henderson, 2013/07/17
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw, Richard Henderson, 2013/07/17
- Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw,
Anthony Liguori <=
[Qemu-devel] [PULL 4/5] hw/alpha: Drop latch_tmp hack, Richard Henderson, 2013/07/14
[Qemu-devel] [PULL 5/5] hw/alpha: Use SRM epoch, Richard Henderson, 2013/07/14