mirage / xen-arm-builder

Archived - the Xen and ARM support in MirageOS has been superseeded by our PVH support - Build an SDcard image for Xen/ARM, for a Cubieboard
ISC License
57 stars 36 forks source link

Added patch to allow access to 4 Cubie* uarts. #61

Closed sprout42 closed 7 years ago

sprout42 commented 8 years ago
  1. Updated clone-repos.sh to look for xen patches
  2. Added cubie* uart blacklist fix. Although it is only a different sort of workaround. The real fix will be to have Xen examine the memory regions allocated to a domain, and blacklist any device that overlaps with the regions Xen is using for itself.
talex5 commented 8 years ago

Is anyone here qualified to review this? Where did these addresses come from?

sprout42 commented 8 years ago

The addresses are from the default device tree in the Linux kernel. Such as this line: https://github.com/talex5/linux/blob/master/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts#L69

Since UART0 is used by Xen as the Xen console (and this cannot be changed at this time), the 4 base UARTs (0, 1, 2, and 3) all reside in the memory region that is owned by Xen, so we cannot allow a guest domain to also access that memory. However, since it is useful to have raw UART access in dom0, this change will allow UARTs 4, 5, 6, and 7 to remain in the DTB used by dom0.

As the commit comment says however, this is just another form of workaround. The proper fix is for Xen to check the memory region of every device in the DTB to confirm that it does not conflict with memory reserved by Xen. The previous driver blacklist did not prevent someone from loading up a DTB with a non-standard device mapped over the UART0 memory area, which would allow that device to be passed through to dom0 and thus be able to read and write to the memory region used by the Xen console.

lcdunstan commented 8 years ago

Do you have a link to a modified sun7i-a20-cubieboard2.dtb (or .dts) including UARTs 4-7? I could edit it manually but it would be better to test with the same one that you used, if possible.

sprout42 commented 8 years ago

I am working on finding it. We did this work about a year ago and then forgot about it until a recent conversation on the mirage-devel mailing list.

I believe that we just manually added the entries for the UARTS based on the addresses in the AllWinner A20 manual: http://dl.linux-sunxi.org/A20/A20%20user%20manual%20v1.3%2020141010.pdf (Section 1.3).

lcdunstan commented 8 years ago

OK, don't worry about it then. I've done a quick tweak to enable a couple of them, so the patch is working for me:

$ find /sys/devices/soc\@01c00000/ -name 'ttyS?'
/sys/devices/soc@01c00000/1c29c00.serial/tty/ttyS1
/sys/devices/soc@01c00000/1c29800.serial/tty/ttyS0
mor1 commented 7 years ago

Hi; Apologies for this having sat so long. Would you be able to test whether the recent refresh using more up-to-date source repos means that this PR is no longer required? If it is, then of course happy to look at an updated one :) (And I promise not to leave it sitting for a year this time...!) I'll close this in the meantime, but do feel free to reopen or issue a new PR as you wish. Thanks!

sprout42 commented 7 years ago

To be honest I forgot that this was still open :)

I'm not sure if the changes in this PR look as strange to you as it does to me? We made some further changes last spring to the https://github.com/dornerworks/xen-arm-builder/ fork to bring the Xen kernel, tools, Linux and U-Boot versions up to date so our master branch has quite a few differences now.

I double checked the latest Xen source and it still blacklists the cubietruck (and cubieboard2) UARTs by driver and not by memory address. So a Xen patch would still be necessary to allow dom0 Linux to access at least a few of the hardware UARTs.

mor1 commented 7 years ago

Ok, thanks. I guess that means still applying https://github.com/mirage/xen-arm-builder/pull/61/files#diff-f20509b79dfec3ab4f2b2d903f9162ed ? I'll take a look at adding a patch of that nature into the refreshed version, but I guess if things had diverged significantly before that anyway, there's indeed no need to revisit this PR anyway. Thanks!