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

driver MMIO virtual address space mapping #26107

Closed
andrewboie opened this issue Jun 10, 2020 · 3 comments · Fixed by #26750
Closed

driver MMIO virtual address space mapping #26107

andrewboie opened this issue Jun 10, 2020 · 3 comments · Fixed by #26750
Assignees
Labels
area: Drivers area: Memory Protection Enhancement Changes/Updates/Additions to existing features priority: medium Medium impact/importance bug
Milestone

Comments

@andrewboie
Copy link
Contributor

andrewboie commented Jun 10, 2020

Is your enhancement proposal related to a problem? Please describe.
All drivers currently assume a physical memory map, and directly read/write MMIO addresses provided by devicetree.

With the introduction of a virtual address space, it's not generally feasible to identity-map MMIO ranges in the page tables. The kernel needs to set up a virtual mapping for driver MMIO ranges, and drivers need to use the virtual address mapping instead of the physical address.

Initial implementations of virtual address spaces will map driver memory at runtime. It may be possible later to establish some driver mappings at build time.

Describe the solution you'd like
This is not a completely fleshed-out spec and I'm hoping to get some feedback from people more experienced with Zephyr drivers and devicetree than I.

What I think I need for runtime mapping is something akin to Linux's ioremap:

void *ioremap(unsigned long phys_addr, unsigned long size);

Ours might be something like:

/**
 * Map a device MMIO region into the address space
 *
 * This function has a side effect such that if an MMU is present,
 * a permanent mapping will be established in kernel memory for the
 * specified I/O range, with read/write access for supervisor
 * mode and caching properties set appropriately.
 *
 * If no MMU is present, this simply returns phys_addr cast to
 * void *.
 *
 * @param phys_addr Physical address of the IO region
 * @param size Size of the IO region
 * @return The virtual address the region is mapped to, in the
 *         kernel's address space
 */
void *device_io_map(uintptr_t phys_addr, size_t size);

Internally this may call an arch_mem_map() function to handle programming the MMU.

Drivers at initialization time will need to invoke this mapping function to map the MMIO range provided by DT into a virtual address and store the returned virtual address somewhere. Any MMIO access will need to use the virtual address instead of the physical address provided by DT.

There must be no footprint or performance overhead for devices that do not use virtual memory. It might be useful to define device_io_map() on such systems as a constant expression returning phys_addr simply cast to void *, but no extra memory must be used to store such mapping.

Using this mechanism should look clean. There shouldn't be a bunch of #ifdefs to add runtime mapping to a driver. Ideally this support should touch individual drivers as little as possible, I'm hoping just a couple lines of code each.

This might require some thought; a lot of drivers store the base address for a particular driver instance in the read-only config struct, but in this case we would need to store the base virtual address obtained at runtime in the driver_data instead (which lives in RAM and not ROM).

This relates to drivers that enumerate at runtime via PCI, since their MMIO ranges are only known at runtime as well and we need to store their base addresses in RAM and not ROM. I'm not sure if we found a general solution for that.

For the work I'm doing for #26088 I'm just hacking this into the few drivers I need to get up and running in QEMU, but need a better solution than that.

@andrewboie andrewboie added Enhancement Changes/Updates/Additions to existing features area: Memory Protection labels Jun 10, 2020
@andrewboie andrewboie self-assigned this Jun 10, 2020
@andrewboie andrewboie added this to the v2.4.0 milestone Jun 10, 2020
@andrewboie andrewboie changed the title driver address space mapping driver MMIO virtual address space mapping Jun 10, 2020
@andrewboie andrewboie added the priority: medium Medium impact/importance bug label Jun 10, 2020
@andrewboie
Copy link
Contributor Author

andrewboie commented Jun 10, 2020

This relates to drivers that enumerate at runtime via PCI, since their MMIO ranges are only known at runtime as well and we need to store their base addresses in RAM and not ROM. I'm not sure if we found a general solution for that.

Looks like there isn't a general solution for this. I looked at the NS16550 driver. What it has is two possible locations for the device configuration, plus a get_port() wrapper to all MMIO access:

/* device config */
struct uart_ns16550_device_config {
	struct uart_device_config devconf;
	...
#ifdef UART_NS16550_PCIE_ENABLED
	bool pcie;
	pcie_bdf_t pcie_bdf;
	pcie_id_t pcie_id;
#endif
};

/** Device data structure */
struct uart_ns16550_dev_data_t {
#ifdef UART_NS16550_PCIE_ENABLED
	struct uart_device_config devconf;
#endif
	...
};
...
static inline uint32_t get_port(struct device *dev)
{
#ifdef UART_NS16550_PCIE_ENABLED
	if (DEV_CFG(dev)->pcie) {
		return DEV_DATA(dev)->devconf.port;
	}
#endif /* UART_NS16550_PCIE_ENABLED */

	return DEV_CFG(dev)->devconf.port;
}

I'd like a general solution to:

  • be an either/or situation with storing the base address in RAM/ROM, and not both RAM & ROM.
  • Have less #ifdefs and generally be more concise, if that's possible through macros or something like that

I'm wondering if there's some kind of macro magic possible like BASE_ADDR(device, member_name) which pulls the address out of dev_data or config appropriately based on configuration.

@andrewboie
Copy link
Contributor Author

andrewboie commented Jun 10, 2020

I'm expecting the actual device_io_map() calls to be invoked from the driver's init function. Currently we setup MMU identity mappings using MMU_BOOT_REGION() macros or runtime calls to z_x86_mmu_region() in SOC code, this needs to all be moved to the drivers themselves.

To establish the mapping in the init function, we will need to know the base address and size of the MMIO region. For most drivers we get this out of DT. Per @dcpleung both the base address and size might be a value we only know at runtime via PCI. I expect we will only need the size value in the init function and will not need to store it, but we'll need to be able to query the PCI subsystem for this data inside the driver init function.

What would be really cool would be a macro like:

	io_addr = DEVICE_MAP(device);

DEVICE_MAP() will:

  1. If this is a PCI device, probe and fetch the base/size from PCI at runtime. Otherwise, get the base/size directly from DT. This will require some way to get the DT node information from the device pointer...I don't know if this is possible
  2. Invoke device_io_map() using the base/size values obtained.

This will let the IO mapping be a nice one-liner in the init function. But I don't know if it's feasible to get the data in, for example, DT_INST(0, ns16550) from the associated device struct for that 16550 instance 🤔

@andrewboie
Copy link
Contributor Author

I think I have a solution for this.

There will be three variants of DEVICE_MMIO macros:

  • One for devices which need to manage one MMIO location
  • One for devices which needs to manage several MMIO locations
  • One for devices which are not using the Zephyr driver model (timers, irq controllers, etc)

For each variant, we will have APIs:

  • Macros to reserve room in dev_data/config_info for MMIO information (or toplevel for the non driver model case)
  • A macro to initialize config_info/ROM information from DTS with MMIO ranges
  • A simple macro used in init functions to map DTS MMIO ranges into the address space and set the runtime MMIO pointer
  • Lower-level APIs for handling less simple cases, such as when the MMIO range needs to be probed out of PCIe -- not as simple to use, but flexible enough for all situations at the expense of more code you have to write to wire everything up
  • An easy to use DEVICE_MMIO_GET() macro which returns the MMIO address (from either ROM or RAM as appropriate)

Attempts at merging VM mapping with PCIe probing/mbar querying ran into too many corner cases, we won't have oneliners for these cases but I think that's OK for now, DEVICE_MMIO_GET() works the same you will just need a few more lines in the init function to control the PCIe stuff.

Patches for this pretty soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Drivers area: Memory Protection Enhancement Changes/Updates/Additions to existing features priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant