Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bootrom functions #17

Closed
thejpster opened this issue Feb 4, 2021 · 11 comments
Closed

Bootrom functions #17

thejpster opened this issue Feb 4, 2021 · 11 comments

Comments

@thejpster
Copy link
Member

Does the bootrom table lookup code work? It's just that according to Datasheet section 2.8.3, the Bootrom stores the table addresses as 16-bit values - these need to be read as 16-bit values, then expanded to 32-bit, then converted to the appropriate pointer. I suspect the code as written will read 32-bits from the bootrom, and get thus get the pointer very wrong.

@AsafFisher
Copy link
Member

Can you refer to the exact line where you see a 32-bit pointer read from the rom?

@AsafFisher
Copy link
Member

AsafFisher commented Feb 5, 2021

We use rom_table_lookup which is supplied by the bootrom, so technically speaking we are not accessing the rom directly. If this function read 16-bit pointer value then we are fine :)

Hopefully it works like that. If not, well no pi pico has ever worked properly :O

@AsafFisher
Copy link
Member

Screen Shot 2021-02-05 at 1 16 51 PM

@thejpster
Copy link
Member Author

thejpster commented Feb 5, 2021

const FUNC_TABLE: *const *const u16 = 0x14 as _;

rom_table_lookup(FUNC_TABLE, some_tag);

fn rom_table_lookup<T>(table: *const *const u16, tag: RomFnTableCode) -> T {
    unsafe {
        let rom_table_lookup: RomTableLookupFn<T> = core::mem::transmute(ROM_TABLE_LOOKUP_PTR);
        rom_table_lookup(*table, u16::from_le_bytes(tag) as u32)
    }
}

The deref of the table pointer with *tablewill read four bytes (a *const _) from memory: 0x14 .. 0x17. You should only read 0x14 and 0x15 and add two zero bytes. You should probably set FUNC_TABLE to be a *const u16, then do a core::ptr::volatile_read(), then cast the u16 to a usize, then pass that to the rom_table_function.

In your ASM listing, I think LDR R0, [R0] will do a 32-bit read from the address 0x14. That LDR should be an LDRH.

But maybe I'm wrong, and this code works fine as-is? Maybe I've read the datasheet wrong?

@thejpster
Copy link
Member Author

Note how the SDK code does a 16-bit read:

// Convert a 16 bit pointer stored at the given rom address into a 32 bit pointer
#define rom_hword_as_ptr(rom_address) (void *)(uintptr_t)(*(uint16_t *)rom_address)

@AsafFisher
Copy link
Member

I think it's the same because jumping to 0x0014 is the same as jumping to 0x00000014 but essentially you are right

@AsafFisher
Copy link
Member

AsafFisher commented Feb 5, 2021

And in thumb all 16 bit are extended to 32 bit register so yah...

I don't have the pico so I can't test it (:

Let me know if I am wrong

@thejpster
Copy link
Member Author

Jumping to 0x0014 is the same as jumping to 0x00000014, but using the ROM table doesn't involve jumping to 0x0014 - it involves reading a two-byte value stored at 0x0014..0x0015.

@AsafFisher
Copy link
Member

I didn't notice the double const*😳

@AsafFisher
Copy link
Member

I am fixing it right now, why should I use core::ptr::volatile_read and not just regular deref?

@thejpster
Copy link
Member Author

I would use it because it is more explicit that you are loading from a specific address in flash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants