IOMMU: Add SDF Support

[cazb2]

This commit adds iommu support to microkit sdfs.

Define a constant to represent the a tool enforced limitation on the maximum virtual address suported for an IO address space. This is due to current x86 IOMMU behaviour in the kernel which dynamically detects the maximum virtual address for an IO address space. For more info read: BOOT_CODE bool_t vtd_init_num_iopts(uint32_t num_drhu) in seL4/src/plat/pc99/machine/intel-vtd.c.

Add PciDevice struct and PCIDeviceParseError enum. This types allow us to implement the Display trait which gives a consistent way to output PCI information. The FromStr trait implemented for PCIDevice avoid duplicating logic required to parse the PCI identifier from the xml. When combined with the display trait on the PCIDeviceParseError, we get consistent error messages and avoid repeating code.

Add IOMMUDeviceIdentifier and IOMMUDeviceIdentifierParseError to support parsing the sdf based on the current architecture. This allows the sdf parsing to easily support ARM SMMU and RISCV IOMMU. The Display traits again remove repeated code when outputting IOMMUDeviceIdentifer or IOMMUDeviceIdentifierParseError, particularly useful for error messages.

Add SysIOMapPerms. Representing the perms in an enum completely removes the existence of invalid IOMapPerms once we parse the sdf.

Add SysIOMap. The dual to SysMap, used to represent a mapping in a devices IO virtual address space.

Replace ExecutionContext trait with a Map trait. The previous trait was primarily used in the check_maps function to handle memory regions in virtual machines and normal pds. This Map trait allowed the one check_maps function to handle both SysIOMaps and SysMaps uniformly. The additional information that was provided by the polymorhpic ExecutionContext is just passed as a string to be used in the respective error message. This is because outside of the check_maps function a protection_domain and a virtual_machine are handled as distinct types.

Add error checking cases to check_maps. If the map_start + mr.size overflows alot of the pre-exisiting logic breaks so check for it. Also check that we don't overflow the top of user_virtual memory accouting for the differences between protection_domains, virtual_machines and io address spaces.

Add check_io_maps adapter. Required because check_maps is designed to error check within one address space. Therefore we collect all the mappings that have been made on a per IO address space basis.

capdl/irq.rs waas using the old pci_bus,pci_dev,pci_func form. Since we have a real type now, we updated the expected match form.

[midnightveil]

For comparison's sake: #467.

[midnightveil]

OK, so so far this is good :)

There's a few relatively minor changes, but I think I'm happy with this format in the SDF.

I don't know if you've yet to do it, but documentation in the manual would be good.

(also like the rest of the implementation, ha).

[cazb2]

OK, so so far this is good :)

There's a few relatively minor changes, but I think I'm happy with this format in the SDF.

I don't know if you've yet to do it, but documentation in the manual would be good.

(also like the rest of the implementation, ha).

Yep, I thought it would be easier to make a few smaller PRs rather than one big one.

[midnightveil]

Ok, last thing:

Can you add this sort of check?

microkit/tool/microkit/src/sdf.rs

Lines 1141 to 1147 in b5cd889

	if !config.hypervisor {
	return Err(value_error(
	xml_sdf,
	node,
	"seL4 has not been built as a hypervisor, virtual machines are disabled".to_string()
	));
	}

So that until the IOMMU is enabled by default it should be impossible to make these.

(It will be based off the KernelHaveIOMMU define, like for hypervisor).

[midnightveil]

Note: this is pending follow-up PRs for functionality, to be merged as a group.

[dreamliner787-9]

Suggested change

	io address spaces. This type of mapping is known as an *iomap*. It supports
	IO address spaces. This type of mapping is known as an *iomap*. It supports

Can you please capitalise this since it's an abbreviation, need to be done elsewhere in the manual too, but leave io_address_space as is.

[cazb2]

should be done now

[midnightveil]

Since you have commits in here that are a refactoring I have to review this at a per-commit level. However, as it stands you have fixup commits that have been added later that mean you change earlier code added. I'd appreciate if you cleaned up the commit history so that each change is standalone and you don't go back and change things you just added. You now have commits that contain changes of several previous commits.

[midnightveil]

Please don't link a google link. Link the static Intel page and the full name of the manual.

[midnightveil]

Please be a bit more formal with your comments. These sentences are very long and run on and could be a bit more brief.

[cazb2]

Since you have commits in here that are a refactoring I have to review this at a per-commit level. However, as it stands you have fixup commits that have been added later that mean you change earlier code added. I'd appreciate if you cleaned up the commit history so that each change is standalone and you don't go back and change things you just added. You now have commits that contain changes of several previous commits.

Right my bad. All new code from when it was reviewed previously is in separate commits from capitalising IO in the manual to the domain ID support. I haven't changed any of the previously reviewed commits but I get what you mean about GitHub being a pain for looking at individual commits.