[patch][rfa]: SID mapper component - wordsize

[patch][rfa]: SID mapper component - wordsize

[Dave Brolley]

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

Hi,

I ran across a problem with the scaling of the upper bound of a mapped 
range when the wordsize feature is used. The specification

[4*1000-1010]

should allow access to the range 4000-4043, however, the upper bound is 
simply being multiplied by the wordsize, resulting in an upper bound of 
4040. If the mapper is accessed using wordsized elements, the problem 
goes unnoticed, since the mapper does not check the ending address of 
the range being accessed. However, if the mapper is accessed one byte at 
a time (as GDB often does), the final 3 bytes are considered unmapped.

Attached is the patch which corrects this. OK to commit?

Dave

[-- Attachment #2: mapper-wordsize.patch.txt --]
[-- Type: text/plain, Size: 1511 bytes --]

Index: component/mapper/compMapper.cxx
===================================================================
RCS file: /cvs/src/src/sid/component/mapper/compMapper.cxx,v
retrieving revision 1.12
diff -c -p -r1.12 compMapper.cxx
*** component/mapper/compMapper.cxx	11 Nov 2002 22:28:28 -0000	1.12
--- component/mapper/compMapper.cxx	13 Feb 2003 23:49:05 -0000
***************
*** 1,6 ****
  // compMapper.cxx - a bus mapper component.  -*- C++ -*-
  
! // Copyright (C) 1999-2002 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
--- 1,6 ----
  // compMapper.cxx - a bus mapper component.  -*- C++ -*-
  
! // Copyright (C) 1999-2003 Red Hat.
  // This file is part of SID and is licensed under the GPL.
  // See the file COPYING.SID for conditions for redistribution.
  
*************** generic_mapper::make_name_mapping (const
*** 774,780 ****
    // scale all values by the word width
    record.mapped_base *= record.bytes_per_word;
    record.low    *= record.bytes_per_word;
!   record.high   *= record.bytes_per_word;
    record.stride *= record.bytes_per_word;
    record.width  *= record.bytes_per_word;
  
--- 774,780 ----
    // scale all values by the word width
    record.mapped_base *= record.bytes_per_word;
    record.low    *= record.bytes_per_word;
!   record.high   = (record.high + 1) * record.bytes_per_word - 1;
    record.stride *= record.bytes_per_word;
    record.width  *= record.bytes_per_word;
  

Re: [patch][rfa]: SID mapper component - wordsize

[Ben Elliston]

>>>>> "Dave" == Dave Brolley <brolley@redhat.com> writes:

  Dave> Attached is the patch which corrects this. OK to commit?

Fine.  Please make sure that the documentation documents the right
behaviour (it probably does).

Ben

Re: [patch][rfa]: SID mapper component - wordsize

[Frank Ch. Eigler]

brolley wrote:

> [...] The specification
> [4*1000-1010]
> should allow access to the range 4000-4043, however, the upper bound
> is simply being multiplied by the wordsize, resulting in an upper
> bound of 4040.

Actually, addresses in the mapper specs are inclusive, not exclusive.
That's why one often sees ranges ending with ...ffff.  Plain raw
scaling such inclusive limits doesn't make sense though, so a special
case calculation like yours would make sense.  However, care should be
taken not to confuse exclusive vs inclusive bounds.

> If the mapper is accessed using wordsized elements, the problem goes
> unnoticed, since the mapper does not check the ending address of the
> range being accessed.

That's a peculiar other bug.  To be consistent with the byte-oriented
addressing across sid::bus, the size of the access would need to be
included in the checks.


- FChE

Re: [patch][rfa]: SID mapper component - wordsize

[Dave Brolley]

Frank Ch. Eigler wrote:

>brolley wrote:
>
>  
>
>>[...] The specification
>>[4*1000-1010]
>>should allow access to the range 4000-4043, however, the upper bound
>>is simply being multiplied by the wordsize, resulting in an upper
>>bound of 4040.
>>    
>>
>
>Actually, addresses in the mapper specs are inclusive, not exclusive.
>That's why one often sees ranges ending with ...ffff.  Plain raw
>scaling such inclusive limits doesn't make sense though, so a special
>case calculation like yours would make sense.  However, care should be
>taken not to confuse exclusive vs inclusive bounds.
>
Right. That's precisely what the patch does. It computes the intended 
inclusive bounds.

>>If the mapper is accessed using wordsized elements, the problem goes
>>unnoticed, since the mapper does not check the ending address of the
>>range being accessed.
>>    
>>
>
>That's a peculiar other bug.  To be consistent with the byte-oriented
>addressing across sid::bus, the size of the access would need to be
>included in the checks.
>  
>
That's a separate bug (wasn't sure if it was intentional or not) and an 
exercise for someone's copious spare time :-)

Dave