On 31/07/14 15:25, Robert Millan wrote:
> Hi,
>
> Please consider this patch. It implements PCI I/O space on Linux by using
> iopl()
> to enable blanket I/O permissions.
Hi Robert,
Almost perfect, but needs a mostly mechanical adjustment. The rumpuser
interface is sort of the "holy grail" of rump kernels, and since it is
widely implemented, changing the interface in any way is not possible
without careful coordination.
Now, since your rumpuser_io_init() is really only used by pci-userspace
code, we don't have to make it part of the main rumpuser interface. You
can add the interface to pci_user.h and then implement the counterpart
in the pci-userspace repository. Please stop to consider at least for a
few moments to make sure the interface is sufficiently, but not overly,
generic. For example, is something like:
rumpcomp_pci_init(int flags, int *caps)
Better than just something with void? Then you could check caps and
enable io space based on the value. Also consider platforms where you
do not have to raise iopl separately (the same rumpcomp_pci interface is
implemented for Xen guests).
> diff --git a/sys/rump/dev/lib/libpci/pci_at_mainbus.c
> b/sys/rump/dev/lib/libpci/pci_at_mainbus.c
> index 934dfa8..14b2688 100644
> --- a/sys/rump/dev/lib/libpci/pci_at_mainbus.c
> +++ b/sys/rump/dev/lib/libpci/pci_at_mainbus.c
> @@ -61,13 +61,14 @@ RUMP_COMPONENT(RUMP_COMPONENT_DEV)
>
> if ((error = rump_vfs_makedevnodes(S_IFCHR, "/dev/pci", '0',
> cmaj, 0, 4)) != 0)
> - printf("pci: failed to create /dev/pci nodes: %d", error);
> + printf("pci: failed to create /dev/pci nodes: %d\n", error);
> }
I assume this is the typo. It's better to submit typo fixes separately
from actual patches, but I applied it to the NetBSD tree (not yet pulled
up to the src-netbsd mirror on github).
> +
> + error = rumpuser_io_init();
> + if (error == 0)
> + pba.pba_flags |= PCI_FLAGS_IO_OKAY;
> + else
> + printf("pci: unable to raise I/O privilege level (error %d)\n",
> error);
Could use aprint_error.
> diff --git a/sys/rump/dev/lib/libpci/rumpdev_bus_space.c
> b/sys/rump/dev/lib/libpci/rumpdev_bus_space.c
> index 9f61bcb..19635df 100644
> --- a/sys/rump/dev/lib/libpci/rumpdev_bus_space.c
> +++ b/sys/rump/dev/lib/libpci/rumpdev_bus_space.c
> @@ -65,7 +65,8 @@ bus_space_read_1(bus_space_tag_t bst, bus_space_handle_t
> bsh,
> uint8_t rv;
>
> if (bst == 0) {
> - panic("8bit IO space not supported");
> + unsigned short addr = bsh + offset;
> + __asm__ __volatile__("inb %1, %0" : "=a"(rv) : "d"(addr));
> } else {
> rv = *(volatile uint8_t *)(bsh + offset);
> }
Can you wrap these in #ifdef x86 #else?
That needs to be done better some day, but I think it's good enough for
now, will still keep things compiling on non-x86.
------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls.
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
_______________________________________________
rumpkernel-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/rumpkernel-users
