On 31/07/14 18:00, Antti Kantee wrote:
> 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)
But the init function itself it quite bus-agnostic. What happens if something
outside of PCI also wants it?
> Better than just something with void? Then you could check caps and enable
> io space based on the value.
I'm not sure what you mean by caps. Are you thinking about IOMMU,
authorization, etc?
> Also consider platforms where you do not have to raise iopl separately (the
> same rumpcomp_pci interface is implemented for Xen guests).
Thanks, I will have a look.
> 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).
Ack!
>
>> +
>> + 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.
Ok.
>
>> 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?
Sure.
--
Robert Millan
------------------------------------------------------------------------------
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
