Re: [Qemu-devel] [RFC][PATCH 11/15] memory: Allow unaligned address_space_rw

[Paolo Bonzini <pbonzini@redhat.com>]

Il 06/05/2013 16:58, Jan Kiszka ha scritto:
> On 2013-05-06 16:55, Paolo Bonzini wrote:
>> Il 06/05/2013 16:26, Jan Kiszka ha scritto:
>>> This will be needed for some corner cases with para-virtual the I/O
>>> ports.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  exec.c |   33 ++++++++++++++++++---------------
>>>  1 files changed, 18 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/exec.c b/exec.c
>>> index 3ee1f3f..9c582b1 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -1833,38 +1833,41 @@ void address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
>>>      uint8_t *ptr;
>>>      uint32_t val;
>>>      MemoryRegionSection *section;
>>> +    MemoryRegion *mr;
>>>  
>>>      while (len > 0) {
>>>          l = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr;
>>>          if (l > len)
>>>              l = len;
>>>          section = address_space_lookup_region(as, addr);
>>> +        mr = section->mr;
>>>  
>>>          if (is_write) {
>>> -            if (!memory_region_is_ram(section->mr)) {
>>> +            if (!memory_region_is_ram(mr)) {
>>>                  hwaddr addr1;
>>>                  addr1 = memory_region_section_addr(section, addr);
>>>                  /* XXX: could force cpu_single_env to NULL to avoid
>>>                     potential bugs */
>>> -                if (l >= 4 && ((addr1 & 3) == 0)) {
>>> +                if (l >= 4 && ((addr1 & 3) == 0 || mr->ops->impl.unaligned)) {
>>
>> Does the length matter at all if unaligned accesses are allowed?  I
>> think it shouldn't...
> 
> What do you mean? The length test here is not about alignment, it's
> about proper split-up depending on the input size (we cannot use 32 bit
> for all accesses and do not want to make them all byte accesses, do we?).

Oh right, I was thinking of my IOMMU tree where I have:

        l = len;
        section = address_space_translate(as, addr, &addr1, &l, is_write);

        if (is_write) {
            if (!memory_region_is_ram(section->mr)) {
                /* XXX: could force cpu_single_env to NULL to avoid
                   potential bugs */
                if (l >= 4 && ((addr1 & 3) == 0)) {

and address_space_translate does:

        *plen = MIN(section->size - addr, *plen);

It should not do this if section->mr->ops.unaligned.  Whoever rebases on top
of the other should keep this in mind.

Paolo