cp_bitfield setbit for a specific field may affect other fields

[ameirson]

The setbit function does not mask the fieldval before shifting it by bit offset. The result is that the shifval will affect adjacent fields if the fieldval is (by mistake) larger than the max field value.

shiftval= fieldval << self.lsb

should be: shiftval= (self.mask & fieldval) << self.lsb

I further suggest a more strict approach which will raise an exception if this happens. Otherwise the test code may not know and hide further erroneous operations.

[bat52]

I agree there should be a check on the fieldval range, but your suggested fix is wrong. I applied a proper check, and raise an error in the latest commit, as I report below.

    shiftval= fieldval << self.lsb
    maskinv= self.mask ^ literal_eval('0xFFFFFFFF')
    regmasked = regval & maskinv
    outregval = regmasked + (shiftval & self.mask)

    # check new value in range
    if (shiftval & self.mask) < shiftval:
        raise ValueError(f'Bitifield value f{fieldval} out of range!')

[ameirson]

Thanks for the quick response !

For sharing with colleagues, how do you suggest I distribute a package / version with a fix ?

Thanks, Alon

On Thu, May 4, 2023 at 11:03 PM Marco Merlin @.***> wrote:

Closed #1 https://github.com/bat52/cheap_pie/issues/1 as completed.

— Reply to this email directly, view it on GitHub https://github.com/bat52/cheap_pie/issues/1#event-9171135659, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARFULSLFOPSS2OUQKO4BVX3XEQDR3ANCNFSM6AAAAAAXWBGEJU . You are receiving this because you authored the thread.Message ID: @.***>

[bat52]

Just because you are my first bug reporter, I have updated the pypi package to version 0.1.17 that includes the fix, so that you can install it with pip as usual. However, in general you can install pypi packages from github: I have added instructions on the readme. Just for my curiosity: which parser and transport layer are you planning to use?

[ameirson]

Hi,

Thanks again. Actually I did figure out quickly how to pip install directly from github. Though I had a problem that the cbitfield file was not overwritten which I resolved by first uninstalling the package. Any thoughts about that ?

Using the ipxact parser and a couple of proprietary transport. The package is not used for standard chip verification as the DV team is using System Verilog under Synopsys VCS and did so before I introduced the cheap-pie package. Perhaps it will become relevant for the next chip, though we will need to weigh in all the legacy verification code already implemented.

One of our transport layers might be of interest - it is a simple register access emulation (ral). Since we are not connecting directly to the DV environment we use a very simple ral to allow implementing and basic testing of some sequences. The ral allows the user to define a list of values that will be returned (one value at a time) whenever a read register is done, returning to the beginning when reaching the end of the list. When a register write is performed the entire list is overwritten, just for coherency of read-after-write returns the write value. Not so useful though. The transport uses the ral and also outputs all register access for review.

If I may point out / ask about a missing feature - support for register "dim". IP-Xact allows register (and registerFile) to have a dimension - an array like interface. I do not see such support under the cheap-pie package.

Thanks, Alon

On Fri, 5 May 2023 at 21:56 Marco Merlin @.***> wrote:

Just because you are my first bug reporter, I have updated the pypi package to version 0.1.17 that includes the fix, so that you can install it with pip as usual. However, in general you can install pypi packages from github: I have added instructions on the readme. Just for my curiosity: which parser and transport layer are you planning to use?

— Reply to this email directly, view it on GitHub https://github.com/bat52/cheap_pie/issues/1#issuecomment-1536649492, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARFULSLN35C4NURBCZZGKLDXEVEOHANCNFSM6AAAAAAXWBGEJU . You are receiving this because you authored the thread.Message ID: @.***>

[bat52]

resolved by first uninstalling the package. Any thoughts about that ?

I usually do the same, but I am sure pip has a proper command to simply update a package.

One of our transport layers might be of interest - it is a simple register access emulation (ral). [...]

I am not sure I understood this all, but it sounds similar to the cp_dummy_transport, that I use for testing the code without connecting to a physical DUT. Whenever a register is written, a dictionary item is created in an internal data structure, so that the same value can be read back for consistency.

if I may point out / ask about a missing feature - support for register "dim". IP-Xact allows register (and registerFile) to have a dimension - an array like interface.

I do not know about this feature, but the times I read IP-XACT, i felt there's too much of it for myself... I am not particularly interested in adding it into ipxact_parse (which is the cheap_pie self-contained IP-XACT parser), but if it is supported by ipyxact ( https://github.com/olofk/ipyxact ) on which is based ipyxact_parse, perhaps we can make it work. Can you provide an example file ?

[ameirson]

Hi,

Regarding the 'dim' support, it is indeed defined in the ipyxact_yaml.py ( https://github.com/olofk/ipyxact/blob/main/ipyxact/ipxact_yaml.py). Since their ipyxact.parse_tree is dynamic based on the yaml file it is generated automatically.

I attach an example xml and simple script that has this output:

==Example of ip-xact register and registerFile Dim Loading== register memmap_example_dim.reg_dim.reg_2 has dim 0 register memmap_example_dim.reg_dim.reg_with_dim has dim 5 register memmap_example_dim.reg_dim.reg_3 has dim 0 registerFile memmap_example_dim.file_reg_dim.CHANNEL has dim 0 register memmap_example_dim.file_reg_dim.breg_2 has dim 0

Regarding the ral emulation I mentioned, the difference from the cp_dummy is that I can prepare a text file that will provide all read values, for example: memmap_example_dim.reg_dim.reg_2:0:0:0:1

The result being that each read to that register will return the next value. With this method, I can implement simple flows that need to wait for a specific value. It also allows testing the flow with different inputs - each as a different ral emulation file. Again, very simple, but useful without a proper HW emulation.

Attaching the code inline in case the file is dropped:

from ipyxact.ipyxact import Component import os

file_in = os.path.join(os.path.dirname(file), r'ipxact_dim_example.xml')

with open(file_in) as f: component = Component() print("==Example of ip-xact register and registerFile Dim Loading==") component.load(file_in)

for memap in component.memoryMaps.memoryMap:
    for ab in memap.addressBlock:
        for reg_file in ab.registerFile:
            print(f'registerFile

{memap.name}.{ab.name}.{reg_file.name} has dim {reg.dim}') for reg in ab.register: print(f'register {memap.name}.{ab.name}.{reg.name} has dim {reg.dim}')

Thanks, Alon

On Sat, May 6, 2023 at 11:22 PM Marco Merlin @.***> wrote:

resolved by first uninstalling the package. Any thoughts about that ?

I usually do the same, but I am sure pip has a proper command to simply update a package.

One of our transport layers might be of interest - it is a simple register access emulation (ral). [...]

I am not sure I understood this all, but it sounds similar to the cp_dummy_transport, that I use for testing the code without connecting to a physical DUT. Whenever a register is written, a dictionary item is created in an internal data structure, so that the same value can be read back for consistency.

if I may point out / ask about a missing feature - support for register "dim". IP-Xact allows register (and registerFile) to have a dimension - an array like interface.

I do not know about this feature, but the times I read IP-XACT, i felt there's too much of it for myself... I am not particularly interested in adding it into ipxact_parse (which is the cheap_pie self-contained IP-XACT parser), but if it is supported by ipyxact ( https://github.com/olofk/ipyxact ) on which is based ipyxact_parse, perhaps we can make it work. Can you provide an example file ?

— Reply to this email directly, view it on GitHub https://github.com/bat52/cheap_pie/issues/1#issuecomment-1537216782, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARFULSKBT5T4LNU5YLJZL3LXE2XHTANCNFSM6AAAAAAXWBGEJU . You are receiving this because you authored the thread.Message ID: @.***>

[bat52]

I attach an example xml and simple script that has this output:

There is no attachment. But I read from the IP-XACT standard the following:

"A register can have a dim element describing the dimension of the register. If dim is not described, then its value defaults to 1."

The description is a bit vague, to be honest. There is only one example present in the standard as follows, but regards a registerFile

<ipxact:registerFile>
<ipxact:name>CHANNEL</ipxact:name>
<ipxact:description>Channel Descriptor Registers</ipxact:description>
<ipxact:dim>4</ipxact:dim>
<ipxact:addressOffset>'h200</ipxact:addressOffset>
<ipxact:range>'h10</ipxact:range>
...
</ipxact:registerFile>

On top of that I have looked at the LEON2 IP-XACT example, and found an interesting case of memory1, having 2 dims...

<ipxact:register>
<ipxact:name>memory1</ipxact:name>
<ipxact:dim>7</ipxact:dim>
<ipxact:dim>4</ipxact:dim>
<ipxact:addressOffset>'h8</ipxact:addressOffset>
<ipxact:size>8</ipxact:size>
<ipxact:volatile>false</ipxact:volatile>
<ipxact:access>read-write</ipxact:access>

So, all in all this 'dim' feature seems not enough well defined to invest time adding it for the time being.

Regarding the ral emulation I mentioned, the difference from the cp_dummy is that I can prepare a text file that will provide all read values, for example: memmap_example_dim.reg_dim.reg_2:0:0:0:1

One option I had considered, but never implemented for lazyness was for cp_dummy_transport to return the reset value of the register when reading a register that was not yet written: it seems that would accomplish your purpose with less code. But you need to specify the reset value in the IP-XACT description itself. I could patch the code to do the change relatively easily.

[ameirson]

Hi,

I have put the files in Google Driver here https://drive.google.com/drive/folders/1_H_fgPtv9zUUWCKu77EZkhsxs4beYTzS?usp=sharing .

That folder also shows a strange phenomenon that has been troubling me for some time. Would appreciate it if you can take a look - I see unexpected hifread when debugging with breakpoints.

Regarding dim support, I am not familiar with LEON2 IP-XACT can you share a link ? Indeed the usage of dim in the snippet you shared looks strange, but I need the entire context. Anyway, support for dim is not of high priority for me.

Thanks, Alon

On Sun, May 7, 2023 at 11:37 AM Marco Merlin @.***> wrote:

I attach an example xml and simple script that has this output:

There is no attachment. But I read from the IP-XACT standard the following:

"A register can have a dim element describing the dimension of the register. If dim is not described, then its value defaults to 1."

The description is a bit vague, to be honest. There is only one example present in the standard as follows, but regards a registerFile

CHANNEL Channel Descriptor Registers 4 'h200 'h10 ... On top of that I have looked at the LEON2 IP-XACT example, and found an interesting case of memory1, having 2 dims... memory1 7 4 'h8 8 false read-write So, all in all this 'dim' feature seems not enough well defined to invest time adding it for the time being. Regarding the ral emulation I mentioned, the difference from the cp_dummy is that I can prepare a text file that will provide all read values, for example: memmap_example_dim.reg_dim.reg_2:0:0:0:1 One option I had considered, but never implemented for lazyness was for cp_dummy_transport to return the reset value of the register when reading a register that was not yet written: it seems that would accomplish your purpose with less code. But you need to specify the reset value in the IP-XACT description itself. I could patch the code to do the change relatively easily. — Reply to this email directly, view it on GitHub , or unsubscribe . You are receiving this because you authored the thread.Message ID: ***@***.***>

[bat52]

I have put the files in Google Driver here

I do not have the rights

Regarding dim support, I am not familiar with LEON2 IP-XACT can you share a link ?

https://accellera.org/downloads/standards/ip-xact https://accellera.org/images/activities/committees/ip-xact/Leon2_1685-2014.zip

[ameirson]

Sorry, please try now:

https://drive.google.com/drive/folders/1_H_fgPtv9zUUWCKu77EZkhsxs4beYTzS?usp=sharing

On Thu, May 11, 2023 at 9:24 PM Marco Merlin @.***> wrote:

I have put the files in Google Driver here

I do not have the rights

Regarding dim support, I am not familiar with LEON2 IP-XACT can you share a link ?

https://accellera.org/downloads/standards/ip-xact

https://accellera.org/images/activities/committees/ip-xact/Leon2_1685-2014.zip

— Reply to this email directly, view it on GitHub https://github.com/bat52/cheap_pie/issues/1#issuecomment-1544480180, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARFULSPTJZYVYMFIHIQX6QTXFUVHRANCNFSM6AAAAAAXWBGEJU . You are receiving this because you authored the thread.Message ID: @.***>

[bat52]

That folder also shows a strange phenomenon that has been troubling me for some time. Would appreciate it if you can take a look - I see unexpected hifread when debugging with breakpoints.

I am surprised your code returns anything that you would expect. It seems to me your code access the internal property _fields of the namedtuple class returned by the parser: as far as my understanding goes there is not such a property, and this should fail. Yet, if I run your code on my machine I obtain the same result, which I honestly have no idea what actually executes.

Here is how I would modify your script:

for reg in hal:
    print(reg)  # executes a read, equivalent to reg.display()
    print('%s: %s' % (reg.regname,hex(reg.addr)))

Which gives the following output:

[0x00000000] READ 0x0
     reg_dim_reg_2[4:0] @         field_1 [5] = 0x0
     reg_dim_reg_2[6:5] @         field_2 [2] = 0x0
       reg_dim_reg_2[7] @         field_3 [1] = 0x0
    reg_dim_reg_2[14:8] @         field_4 [7] = 0x0
   reg_dim_reg_2[21:15] @         field_5 [7] = 0x0
   reg_dim_reg_2[28:22] @         field_6 [7] = 0x0
reg_dim_reg_2: 0x0
[0x00000004] READ 0x0
reg_dim_reg_with_dim[0] @              f1 [1] = 0x0
reg_dim_reg_with_dim[1] @              f2 [1] = 0x0
reg_dim_reg_with_dim[2] @              f3 [1] = 0x0
reg_dim_reg_with_dim[3] @              f4 [1] = 0x0
reg_dim_reg_with_dim: 0x4
[0x00000020] READ 0x0
    reg_dim_reg_3[31:0] @             PN [32] = 0x0
reg_dim_reg_3: 0x20
[0x00000404] READ 0x0
 file_reg_dim_breg_2[0] @          locked [1] = 0x0
file_reg_dim_breg_2[5:1] @       fsm_state [5] = 0x0
file_reg_dim_breg_2: 0x404

[ameirson]

Hi Marco,

I think you are missing the point. The problem reproduces also without using the _fields. Even if I replace that code with a print and set a breakpoint there it happens. If I cannot resolve this issue, using the package will not allow proper debugging, and may cause people not to use it.

import os from cp_dummy_transport_with_log import cp_dummy from cheap_pie.parsers.ipyxact_parse import ipxact_parse

xml_file = os.path.join(os.path.dirname(file), "ipxact_dim_example.xml")

hif = cp_dummy() # Dummy with Logs at hifread & hifwrite hal = ipxact_parse(fname=xml_file, hif=hif)

print(f'The problem happens when you set a breakpoint at this line')

Thanks, Alon

On Thu, May 11, 2023 at 11:58 PM Marco Merlin @.***> wrote:

That folder also shows a strange phenomenon that has been troubling me for some time. Would appreciate it if you can take a look - I see unexpected hifread when debugging with breakpoints.

I am surprised your code returns anything that you would expect. It seems to me your code access the internal property _fields of the namedtuple class returned by the parser: as far as my understanding goes, there is not such a property, and this should fail. Yet, if I run your code on my machine I obtain the same result, which I honestly have no idea what actually executes.

Here is how I would modify your script:

for reg in hal: print(reg) # executes a read, equivalent to reg.display() print('%s: %s' % (reg.regname,hex(reg.addr)))

Which gives the following output:

[0x00000000] READ 0x0 reg_dim_reg_2[4:0] @ field_1 [5] = 0x0 reg_dim_reg_2[6:5] @ field_2 [2] = 0x0 reg_dim_reg_2[7] @ field_3 [1] = 0x0 reg_dim_reg_2[14:8] @ field_4 [7] = 0x0 reg_dim_reg_2[21:15] @ field_5 [7] = 0x0 reg_dim_reg_2[28:22] @ field_6 [7] = 0x0 reg_dim_reg_2: 0x0 [0x00000004] READ 0x0 reg_dim_reg_with_dim[0] @ f1 [1] = 0x0 reg_dim_reg_with_dim[1] @ f2 [1] = 0x0 reg_dim_reg_with_dim[2] @ f3 [1] = 0x0 reg_dim_reg_with_dim[3] @ f4 [1] = 0x0 reg_dim_reg_with_dim: 0x4 [0x00000020] READ 0x0 reg_dim_reg_3[31:0] @ PN [32] = 0x0 reg_dim_reg_3: 0x20 [0x00000404] READ 0x0 file_reg_dim_breg_2[0] @ locked [1] = 0x0 file_reg_dim_breg_2[5:1] @ fsm_state [5] = 0x0 file_reg_dim_breg_2: 0x404

— Reply to this email directly, view it on GitHub https://github.com/bat52/cheap_pie/issues/1#issuecomment-1544670591, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARFULSKXA22CYEJ6NZ6VF5TXFVHHZANCNFSM6AAAAAAXWBGEJU . You are receiving this because you authored the thread.Message ID: @.***>

[ameirson]

Hi Marco,

I just thought of a possible cause for this and am going to test it. It might be the debugger attempting to present values for "Locals" which triggers the repr of a cp_register, which then calls the hifread.

Well update when I have tested it.

Thanks, Alon

On Fri, May 12, 2023 at 7:53 AM Alon Meirson @.***> wrote:

Hi Marco,

I think you are missing the point. The problem reproduces also without using the _fields. Even if I replace that code with a print and set a breakpoint there it happens. If I cannot resolve this issue, using the package will not allow proper debugging, and may cause people not to use it.

import os from cp_dummy_transport_with_log import cp_dummy from cheap_pie.parsers.ipyxact_parse import ipxact_parse

xml_file = os.path.join(os.path.dirname(file), "ipxact_dim_example.xml")

hif = cp_dummy() # Dummy with Logs at hifread & hifwrite hal = ipxact_parse(fname=xml_file, hif=hif)

print(f'The problem happens when you set a breakpoint at this line')

Thanks, Alon

On Thu, May 11, 2023 at 11:58 PM Marco Merlin @.***> wrote:

That folder also shows a strange phenomenon that has been troubling me for some time. Would appreciate it if you can take a look - I see unexpected hifread when debugging with breakpoints.

I am surprised your code returns anything that you would expect. It seems to me your code access the internal property _fields of the namedtuple class returned by the parser: as far as my understanding goes, there is not such a property, and this should fail. Yet, if I run your code on my machine I obtain the same result, which I honestly have no idea what actually executes.

Here is how I would modify your script:

for reg in hal: print(reg) # executes a read, equivalent to reg.display() print('%s: %s' % (reg.regname,hex(reg.addr)))

Which gives the following output:

[0x00000000] READ 0x0 reg_dim_reg_2[4:0] @ field_1 [5] = 0x0 reg_dim_reg_2[6:5] @ field_2 [2] = 0x0 reg_dim_reg_2[7] @ field_3 [1] = 0x0 reg_dim_reg_2[14:8] @ field_4 [7] = 0x0 reg_dim_reg_2[21:15] @ field_5 [7] = 0x0 reg_dim_reg_2[28:22] @ field_6 [7] = 0x0 reg_dim_reg_2: 0x0 [0x00000004] READ 0x0 reg_dim_reg_with_dim[0] @ f1 [1] = 0x0 reg_dim_reg_with_dim[1] @ f2 [1] = 0x0 reg_dim_reg_with_dim[2] @ f3 [1] = 0x0 reg_dim_reg_with_dim[3] @ f4 [1] = 0x0 reg_dim_reg_with_dim: 0x4 [0x00000020] READ 0x0 reg_dim_reg_3[31:0] @ PN [32] = 0x0 reg_dim_reg_3: 0x20 [0x00000404] READ 0x0 file_reg_dim_breg_2[0] @ locked [1] = 0x0 file_reg_dim_breg_2[5:1] @ fsm_state [5] = 0x0 file_reg_dim_breg_2: 0x404

— Reply to this email directly, view it on GitHub https://github.com/bat52/cheap_pie/issues/1#issuecomment-1544670591, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARFULSKXA22CYEJ6NZ6VF5TXFVHHZANCNFSM6AAAAAAXWBGEJU . You are receiving this because you authored the thread.Message ID: @.***>

[ameirson]

Hi Marco,

That was quick. Printing the call stack using traceback revealed that indeed the repr was invoked from the debugger.

[image: image.png]

This allows we to workaround redundant hifread by the debugger by examining the stack. Even though this is done by debugger, It causes unexpected behaviour for the user of your package. I would suggest providing the option that the repr (of register and field) will not involve a hifread. To emphasize the repr uses a "shadow" value rather that an actual hifread, some asterisk or note can be appended to the value.

Thanks, Alon

On Fri, May 12, 2023 at 4:14 PM Alon Meirson @.***> wrote:

Hi Marco,

I just thought of a possible cause for this and am going to test it. It might be the debugger attempting to present values for "Locals" which triggers the repr of a cp_register, which then calls the hifread.

Well update when I have tested it.

Thanks, Alon

On Fri, May 12, 2023 at 7:53 AM Alon Meirson @.***> wrote:

Hi Marco,

I think you are missing the point. The problem reproduces also without using the _fields. Even if I replace that code with a print and set a breakpoint there it happens. If I cannot resolve this issue, using the package will not allow proper debugging, and may cause people not to use it.

import os from cp_dummy_transport_with_log import cp_dummy from cheap_pie.parsers.ipyxact_parse import ipxact_parse

xml_file = os.path.join(os.path.dirname(file), "ipxact_dim_example.xml")

hif = cp_dummy() # Dummy with Logs at hifread & hifwrite hal = ipxact_parse(fname=xml_file, hif=hif)

print(f'The problem happens when you set a breakpoint at this line')

Thanks, Alon

On Thu, May 11, 2023 at 11:58 PM Marco Merlin @.***> wrote:

That folder also shows a strange phenomenon that has been troubling me for some time. Would appreciate it if you can take a look - I see unexpected hifread when debugging with breakpoints.

I am surprised your code returns anything that you would expect. It seems to me your code access the internal property _fields of the namedtuple class returned by the parser: as far as my understanding goes, there is not such a property, and this should fail. Yet, if I run your code on my machine I obtain the same result, which I honestly have no idea what actually executes.

Here is how I would modify your script:

for reg in hal: print(reg) # executes a read, equivalent to reg.display() print('%s: %s' % (reg.regname,hex(reg.addr)))

Which gives the following output:

[0x00000000] READ 0x0 reg_dim_reg_2[4:0] @ field_1 [5] = 0x0 reg_dim_reg_2[6:5] @ field_2 [2] = 0x0 reg_dim_reg_2[7] @ field_3 [1] = 0x0 reg_dim_reg_2[14:8] @ field_4 [7] = 0x0 reg_dim_reg_2[21:15] @ field_5 [7] = 0x0 reg_dim_reg_2[28:22] @ field_6 [7] = 0x0 reg_dim_reg_2: 0x0 [0x00000004] READ 0x0 reg_dim_reg_with_dim[0] @ f1 [1] = 0x0 reg_dim_reg_with_dim[1] @ f2 [1] = 0x0 reg_dim_reg_with_dim[2] @ f3 [1] = 0x0 reg_dim_reg_with_dim[3] @ f4 [1] = 0x0 reg_dim_reg_with_dim: 0x4 [0x00000020] READ 0x0 reg_dim_reg_3[31:0] @ PN [32] = 0x0 reg_dim_reg_3: 0x20 [0x00000404] READ 0x0 file_reg_dim_breg_2[0] @ locked [1] = 0x0 file_reg_dim_breg_2[5:1] @ fsm_state [5] = 0x0 file_reg_dim_breg_2: 0x404

— Reply to this email directly, view it on GitHub https://github.com/bat52/cheap_pie/issues/1#issuecomment-1544670591, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARFULSKXA22CYEJ6NZ6VF5TXFVHHZANCNFSM6AAAAAAXWBGEJU . You are receiving this because you authored the thread.Message ID: @.***>

[bat52]

Discussion on the extra reads continues here https://github.com/bat52/cheap_pie/issues/2