üApp: picar-s: move low-level I2C driver HMAC appending into i2c-ioaccess uobject

This task is the final piece of the I2C driver uobject construction and will move the HMAC appending logic from i2c-bcm2708 low-level I2C driver into the i2c-ioaccess uobject. The following are the proposed sub-tasks (we will expand on the bullets as we proceed):

  1. We will be working with forks and respective branches of the following repositories:

    • uberspark/uberxmhf.git on branch develop
    • uberspark/uberspark.git on branch develop
    • uberspark/uobjcoll-raspberrypi-linux-i2c-bcm2835.git on branch uobjcoll-4.4.y

    Ensure the aforementioned forks/branches are upto date with the upstream repositories. The following gives an example of how to sync your fork of uberspark/uberxmhf.git ; the same can be applied to other repositories. Note these commands have to be applied on a local checkout of your fork.

    git remote add upstream https://github.com/uberspark/uberxmhf.git
    git fetch -a upstream  
    git checkout develop 
    git pull upstream develop 
    git push origin develop
    
  2. Build and install micro-hypervisor and I2C drivers i2c-bcm2708 from the above forks/branches and ensure baseline line-following functionality works.

  3. At a high level, we need to convert code fragments that read from and write to bi->msg->buf to stand-alone constructs that read from and write to statically allocated shadow message buffers corresponding to bi->msg->buf (there seem to be only 3 locations where this happens in the code). Then at the end of bcm2708_i2c_master_xfer before returning we compute the HMAC on the shadow message buffer (instead of msgs->buf) and then copy the shadow message buffer to msgs->buf+msg_size.

  4. The separation achieved in task 3 above will allow us to move the statically allocated shadow message buffers, the fragments operating on the statically allocated shadow message buffers, and the HMAC computation entirely into the i2c-ioaccess uobject .

I think we can begin to make progress for task 3 by focusing on bcm2708_i2c_master_xfer within i2c-bcm2708 and checking to see if we have bi->nmsgs > 1 during our line-following. If so, this means multiple I2C transactions are present. In that case, let us see if all those transactions correspond to the same slave address (i.e, bi->msg[i].addr == PICAR_I2C_ADDRESS for i=0 to bi->nmsgs) and if they are only reads and the maximum value for bi->nmsgs during the line-following.

If for the entire line-following if the I2C messages we see are only for the PICAR_I2C_ADDRESS and just reads then I think we can just allocate a static shadow message buffer of size bi->nmsgs (obtained in the experiment above) and then use that instead of bi->msg[i]->buf within the stand-alone constructs in (3).

Let me know your thoughts and let us discusss details as we dive into this. I propose we aim to get (3) working within i2c-bcm2708 before moving onto (4) to move things into the micro-hypervisor/uobject. This will allow us to use printk within i2c-bcm2708 for debugging while experimenting.

PRs:

Merges:

I updated the toolchains and now have an issue while building the hypervisor.
Reinstalled uberspark, but still see an error:

uberspark >> [docker] Linking uobjcoll.exe …
uberspark >> [docker] arm-linux-gnueabihf-ld: cannot find /home/uberspark/uobjcoll/_triage/uberspark/uobjcoll/platform/rpi3/uxmhf/uobjs/main/uapps/uapp-i2c-ioaccess.o: No such file or directory
uberspark >> [docker] arm-linux-gnueabihf-ld: cannot find /home/uberspark/uobjcoll/_triage/uberspark/uobjcoll/platform/rpi3/uxmhf/uobjs/main/core/hypvtablestubs_data.o: No such file or directory
uberspark >> > invoke_bridge: end(l_retval=false)…
uberspark >> ERROR: error in invoking action bridge!
uberspark >> ERROR: Could not process action!
make: *** [Makefile:27: build] Error 1

Can you post the full build log please?

Here is the file:
uxmhf-build.log (242.8 KB)

Thanks. Pushed changes to upstream uberxmhf.git on develop to fix the build issues. Can you please pull from upstream uberxmhf.git on develop and try again? You don’t need to reinstall the uberspark toolchain.

It succeeds now:

uberspark >> Action processed successfully
uberspark >> uobjcoll processed successfully!
uberspark >> manifest processed succesfully!
built xmhf-rpi3 successfully!

Thanks!

We always have bi->nmsgs equal to 0:
bi->nmsgs: 0
We have two addresses that the driver accesses:
[ 253.268230] msgs->addr: 64
[ 253.271422] msgs->addr: 17

The first one is the servo and the second one is the line following sensor (0x11 hex = 17 decimal)

We do reads from the line sensor and writes to the actuators.

Hmmm, bi->nmsgs should be at least 1 to have a successful transaction within bcm2708_i2c_master_xfer. Are you printing the value right after line 471 as below?

The above line is where bi->nmsgs is initialized to num which is passed to bcm2708_i2c_master_xfer and contains the number of I2C messages for the transaction (including reads and writes to various addresses). Note that bi->nmsgs will be set to 0 after the transaction is processed and control returns back from bcm2708_i2c_master_xfer

Yes, I am aware that it sets it to 0, so had to print it at the right place instead and our messages are always 1:
bi->nmsgs: 1

If this is the case, then it simplifies our design and implementation. We can just have a single static shadow read buffer for the picar-s address (0x17) and just focus on the stand-alone construction for reads to 0x17 as mentioned in the OP (task-3).

I am trying to see what we need to do here.
The struct i2c_msg *msgs is passed to function bcm2708_i2c_master_xfer() and is expected to be populated with data from the read transaction.
The caller expects to have this buffer updated.
What is the proposed change?

Please re-read OP task-3 description and let me know if you have further questions. In light of our new findings, wewill only be focusing on reads to address 0x17 in the context of OP task-3 description

I did read it and this can be done of course as an interim step.
Do we need just a static global buffer in this file for this step?

Yep, that is exactly what I am thinking.

ok, so I have an implementation which works with a static buffer.
Maybe the best thing is to check in the code in my fork. This is my origin:


Code is checked in the uobjcoll-4.4.y branch of my fork.

Ok, I think this is going in the right direction, but instead of setting msgs->buf to static_buffer as in line 465 below, the idea would be to use static_buffer directly whereever there is a reference to bi->msg->buf. There is currently only one such reference in bcm2708_bsc_fifo_drain for reads. So we could possibly check if its the line-follower address and use static_buffer there instead.

Then at the end of xfer control routine, copy this static buffer contents into msgs->buf.

This will help us isolate the places where we use static_buffer as a stand-alone fragment of code that we can then try to move into a uobject.

Hopefully that makes sense. Thanks!

No problem. I changed the implementation as suggested.
The important thing is that both implementation used the static buffer for the actual message processing throughout the call and both worked fine with the car and line following.
Code is checked in : branch uobjcoll-4.4.y in my fork.

Great stuff @antonhristozov!

Is there a way we can move the if(bi->msg->addr == PICAR_I2C_ADDRESS){ on line 213 outside of the while loop? i.e., first do the check and then carry out the while loop within that if block? I am guessing that would still preserve functionality.

If we can do this, then we can combine multiple reads within the same hypercall when we migrate to a uobject…

Yes, we can make this change. Let me try it and retest.