io: Add support for address translation from driver physical address to device physical address

[iuliana-prodan]

There are SoCs that have different memory maps for device and driver. Therefore the shared memory between them has different values and we need to convert the addresses.

Add support to convert a driver physical address to device physical address.

[iuliana-prodan]

I don't know if I shoud fix these complience check errors:

since I've used the same comment style as in the rest of the file.

[iuliana-prodan]

@arnopo @edmooring The CI is failing because I've changed the signature of metal_io_init().
I can make it backwards compatible by creating an extended `metal_io_init()':

• create metal_io_init_extended() with const metal_phys_addr_t *physmap_drv as argument

void metal_io_init_extended (struct metal_io_region *io, void *virt,
	                                       const metal_phys_addr_t *physmap, size_t size,
	                                       const metal_phys_addr_t *physmap_drv,
	                                       unsigned int page_shift, unsigned int mem_flags,
	                                        const struct metal_io_ops *ops)

• the existing metal_io_init() remains as before and calls metal_io_init_extended() with NULL physmap_drv.

void metal_io_init(struct metal_io_region *io, void *virt,
	      const metal_phys_addr_t *physmap, size_t size,
	      unsigned int page_shift, unsigned int mem_flags,
	      const struct metal_io_ops *ops) {
              
             return metal_io_init_extended (io, virt, physmap, size, NULL, page_shift, mem_flags, ops);
}

OR

A second solution would be to create a function that only sets the physmap_drv like:

void set_physmap_drv(struct metal_io_region *io, const metal_phys_addr_t *physmap_drv)
{
	io->physmap_drv= physmap_drv;
}

and will be called after metal_io_init() when we need to do an addr transaltion.

What do you think?

LE: added second option

[arnopo]

Here you could create some regressions. This function is used to return a virtual address based on the physical address, not the remote physical address. This function is used for the buffers, but also the vrings (which use local physmap).

It seems that it would be better to provide a metal_io_ops::phys_to_offset that would point to a metal_io_remote_phys_to_offsetor a platform specific function, ensuring that the same IO region is not used for your vrings and the buffers in the vrings.

[arnopo]

This will break the existing API in the next release. When possible, we try to use a deprecation mechanism.

Here we also have the increase of th function parameters, which seems already quite significant to me. So perhaps it is a good time to rework this API.

I would create a metal_io_config structure that would contain these parameters. Then, create a new function void metal_io_init_region(struct metal_io_region *io, const struct metal_io_config *cfg). Finally, I would update metal_io_init to use metal_io_init_region and tag metal_io_init_region as deprecated.

But perhaps someone else as an alternative to propose?

[tnmysh]

This looks like good idea. Advantage is we can extend structure as needed but keep API interface the same.

[iuliana-prodan]

@arnopo @tnmysh I've decided to add 2 new members: metal_io_ops::remote_phys_to_offset and metal_io_ops::offset_to_remote_phys.

The changes in libmetal will be similar to this commit: a90bbd8

For context see also this comment.

[arnopo]

Do you need to create 2 new ops? Or could you reuse phys_to_offset and offset_to_phys?

Look to me that based on the io address you could decide to apply or not the memory translation

[iuliana-prodan]

Yes, you're right @arnopo. I can use those.
Therefore, I think this PR can be closed.

This can be added to the backlog, as an improvement, for later.

[arnopo]

it could be the device physmap if used on driver side. to rename (remote_physmap?)

[tnmysh]

The difference between io->physmap and io->physmap_drv (or new name) should be documented in comment.

[arnopo]

@arnopo @edmooring The CI is failing because I've changed the signature of metal_io_init(). I can make it backwards compatible by creating an extended `metal_io_init()':

• create metal_io_init_extended() with const metal_phys_addr_t *physmap_drv as argument

void metal_io_init_extended (struct metal_io_region *io, void *virt,
	                                       const metal_phys_addr_t *physmap, size_t size,
	                                       const metal_phys_addr_t *physmap_drv,
	                                       unsigned int page_shift, unsigned int mem_flags,
	                                        const struct metal_io_ops *ops)

• the existing metal_io_init() remains as before and calls metal_io_init_extended() with NULL physmap_drv.

void metal_io_init(struct metal_io_region *io, void *virt,
	      const metal_phys_addr_t *physmap, size_t size,
	      unsigned int page_shift, unsigned int mem_flags,
	      const struct metal_io_ops *ops) {
              
             return metal_io_init_extended (io, virt, physmap, size, NULL, page_shift, mem_flags, ops);
}

OR

A second solution would be to create a function that only sets the physmap_drv like:

void set_physmap_drv(struct metal_io_region *io, const metal_phys_addr_t *physmap_drv)
{
	io->physmap_drv= physmap_drv;
}

and will be called after metal_io_init() when we need to do an addr transaltion.

What do you think?

LE: added second option

I saw this comment only after my review. I proposed a solution in my comments, but regarding my comment in metal_io_phys_to_virt, I wonder if some update in metal_io_init is mandatory. Perhaps just implementing metal_io_ops::phys_to_offset to do the translation would be sufficient.

[tnmysh]

this comment is same as above physmap. The difference should be stated in new comment.