Re: user-level drivers
Hi, On Tue, Nov 15, 2011 at 12:02:58AM +0100, Samuel Thibault wrote: > Considering the target of user-level drivers, I've come to a few > biting points: > > - we should have a PCI arbitrer in the end. - that arbitrer should > permit to confine drivers to resources of only one PCI device, no more > (though of course with DMA, without IO-MMU, you could access anywhere > on the machine, but that's way less probable, and still the arbitrer > might be able to govern them). Yes, that's what I had in mind all along: a bus driver that presents a node for each attached device, giving the driver using this node access to exactly the resources it needs for the device in question. However, I don't expect that to be implemented any time soon... > - so the user-level interface we are discussing at the moment will > probably just be replaced at that moment. I'm not sure it will change all that much? > - so do we really need to polish it yet more, or else simply start > using it, and see what's broken in it? Well, depends what you mean exactly. The physical memory allocation interface in the original branch is just wrong, and clearly not acceptable for mainline IMHO. As for the interrupt stuff, we discussed a number of variants, and I'm not sure which one is best. I think I liked your proposal most (reenabling IRQ on reply to the notification message); but I guess I wouldn't mind sticking with the currently implemented interface either... (Although it is clear that it *will* have to change when implementing a bus driver -- while your variant should be fine in that case as well I think.) -antrik-
Re: user-level drivers
Hello, Considering the target of user-level drivers, I've come to a few biting points: - we should have a PCI arbitrer in the end. - that arbitrer should permit to confine drivers to resources of only one PCI device, no more (though of course with DMA, without IO-MMU, you could access anywhere on the machine, but that's way less probable, and still the arbitrer might be able to govern them). - so the user-level interface we are discussing at the moment will probably just be replaced at that moment. - so do we really need to polish it yet more, or else simply start using it, and see what's broken in it? Samuel
Re: user-level drivers
Hi, On Tue, Aug 23, 2011 at 03:22:06AM +0200, Samuel Thibault wrote: > > > What about using the I/O port scheme? That is, decide_intr_notify > > > doesn't enable IRQ notifications, but instead just returns a > > > handle (compare i386_io_perm_create) that is then passed to > > > device_irq_enable to enable/disable IRQ notifications (compare > > > i386_io_perm_modify). Does that make sense in this IRQ scenario? > > I'm having a look at this thread again, and wondering (once more, I > have to say I don't know so much about the Mach IPC): couldn't the irq > enabling simply done by an answer to the notify? Or are notifies never > replied to? I don't think there is a formal notion of "notifies" in Mach. It's just a kernel-initiated RPC -- similar to those in the pager interface. Anyways, this scheme would require the kernel to create a reply port, along with an appropriate session structure so it knows what to do when the reply arrives. This is probably still simpler to implement though than the handle scheme described above... And the possible overhead from allocating the extra reply port is probably negligible, as this happens directly in the kernel. Another thing perhaps worth considering is that some day we might decide to support shared interrupts. One way of implementing that is to notify the possible drivers one after the other. (Apparently that's what Minix3 does in the end.) In that case we would actually need the reply anyways, so the driver can tell the kernel whether it feels responsible for the interrupt and has acknowledged it in the device, so the kernel can reenable the IRQ line and be done with it; or whether it has to notify the next driver instead. Not sure though whether it's worthwhile considering this possible future use case right now. If we ever do actually implement shared interrupts, it's perfectly possible that we decide actually to go with a different scheme, where the decision which driver is responsible is performed in-kernel with some kind of bytecode... I guess it's a case of YAGNI. -antrik-
Re: user-level drivers
olafbuddenha...@gmx.net, le Tue 24 May 2011 12:09:08 +0200, a écrit : > > What about using the I/O port scheme? That is, decide_intr_notify > > doesn't enable IRQ notifications, but instead just returns a handle > > (compare i386_io_perm_create) that is then passed to device_irq_enable > > to enable/disable IRQ notifications (compare i386_io_perm_modify). > > Does that make sense in this IRQ scenario? > > I'm tempted to cry "don't overengineer" again... Though I must admit > that -- unlike for I/O ports (where it's totally pointless IMHO) -- this > would actually make sense here: the IRQ enabling is a pretty > time-critical operation (happening on every interrupt received); so it > would be good if a bus manager is able to hand out an enable port to > unprivileged driver processes. I'm having a look at this thread again, and wondering (once more, I have to say I don't know so much about the Mach IPC): couldn't the irq enabling simply done by an answer to the notify? Or are notifies never replied to? Samuel
Re: user-level drivers
Hi, On Mon, May 09, 2011 at 11:15:15AM +0200, Thomas Schwinge wrote: > On Mon, 9 May 2011 00:07:16 +0200, Samuel Thibault > wrote: > > /* Requesting IRQ events on a port */ > > > > routine device_intr_notify( > >master_port : mach_port_t; > >in irq : int; > >in id : int; > >in flags : int; > >in receive_port: mach_port_send_t > >); > Shouldn't all the other ``in data'' items be wrapped in an > (architecured-dependent) structure similar to what we're doing for I/O > ports? Compare io_perm_t in i386/include/mach/i386/mach_i386.defs, > i386/include/mach/i386/mach_i386_types.h, i386/i386/io_perm.h. I believe > this would help to separate implementation details (IRQ number being an > integer; specific values of flags; etc.) from the RPC mechanism. Don't overengineer :-) I doubt that there are -- or will ever be -- any architectures where the IRQ line is not an integer... Also, as I pointed out in the past, we are not bound by RPC interfaces forever: we can simply add new ones if the original turns out too limited after all. (BTW, I have no idea what the other two parameters mean...) > What about using the I/O port scheme? That is, decide_intr_notify > doesn't enable IRQ notifications, but instead just returns a handle > (compare i386_io_perm_create) that is then passed to device_irq_enable > to enable/disable IRQ notifications (compare i386_io_perm_modify). > Does that make sense in this IRQ scenario? I'm tempted to cry "don't overengineer" again... Though I must admit that -- unlike for I/O ports (where it's totally pointless IMHO) -- this would actually make sense here: the IRQ enabling is a pretty time-critical operation (happening on every interrupt received); so it would be good if a bus manager is able to hand out an enable port to unprivileged driver processes. OTOH, this is another thing that can be added (with a new interface) when it's needed, i.e. when someone steps up to actually implement a bus manager... I wouldn't bother for now. -antrik-
Re: user-level drivers
Hi, On Mon, May 09, 2011 at 12:07:16AM +0200, Samuel Thibault wrote: > I've started having a look at Zheng Da's user-level driver integration. > I've cleaned his tree a bit, and now considering adding patches to > the debian packages for wider testing. Great :-) The ArchHurd folks have been on it for a while BTW; but haven't managed to successfully build the DDE drivers so far... > /* > * This routine is created for allocating DMA buffers. > * We are going to get a contiguous physical memory > * and its physical address in addition to the virtual address. > */ > routine vm_allocate_contiguous( > host_priv : host_priv_t; > target_task : vm_task_t; > out vaddr : vm_address_t; > out paddr : vm_address_t; > size: vm_size_t); This interface is only acceptable as a proof-of-concept. For upsteam inclusion, we need to do it properly: the RPC should return a special memory object (similar to device_map() ), which can then be mapped into the process address space with vm_map() like any other memory object. Apart from avoiding some redundant code, and being more elegant, this will enable all the goodies possible with memory objects: including various options for the virtual mapping (such as requesting a fixed address); and the possibility to pass the memory object to other processes. (That way a manager process could do a limited number of allocations, and hand the buffers out to unpriviledged driver processes; and/or the driver processes could hand them out to user processes for zero-copy buffer management.) I pointed this out to Zheng Da more or less immediately when he originally implemented the interface above; and basically he agreed that the memory object interface would be better -- but never actually implemented it... This is one of the reasons why I haven't considered the DDE stuff ready for upstream inclusion myself. Another reason is that there is no interface for the userspace drivers to disable the corresponding kernel drivers, to avoid conflicts -- which is another thing that was discussed, but not implemented yet... Also, the userspace code needs various cleanups. (Most notably, we need a more proper build system for libdde_linux26 and the actual drivers.) But of course that shouldn't prevent the kernel bits from being merged (once the proper interfaces are implemented) -- which would actually make my life much easier, as I could use standard gnumach packages again :-) > /* > * enable/disable the specified irq. > */ > routine device_irq_enable( >master_port : mach_port_t; >irq : int; >status : char); > > Should this be rather split into device_irq_enable/disable and drop the > status paramter? Is the disable part actually used at all? AIUI, the kernel IRQ handler should always disable the line; and the userspace driver only has to reenable it, after acknowledging and handling the interrupt... -antrik-
Re: user-level drivers
On Tue, May 10, 2011 at 03:26:26AM +0200, Samuel Thibault wrote: > What do you mean by "phase"? Offset from the alignment. But I don't think it's useful at all in this case. Minimum and maximum addresses and alignment should do. Maybe boundary crossing but usually, specifying the right alignment and size is enough. -- Richard Braun
Re: user-level drivers
On Tue, May 10, 2011 at 03:29:11AM +0200, Samuel Thibault wrote: > There are archs which have varying page size. I'd rather avoid > introducing the page size, can't mig cope with 64bit values on 32bit > archs? Right. Even mmap2() forces multiples of 4096 and not the page size. So using 64 bits seems reasonable. Mig should have no problem with 64 bits values. -- Richard Braun
Re: user-level drivers
Richard Braun, le Mon 09 May 2011 13:33:37 +0200, a écrit : > On Mon, May 09, 2011 at 01:27:32PM +0200, Thomas Schwinge wrote: > > > I'd suggest using natural_t (or unsigned long) too. But then, it can't > > > be used to address >4 GiB physical memory. Consider expressing physical > > > memory in page frame numbers. > > > > Good idea! But: what about differently sized frames (4 KiB/2 > > MiB/whatever amd64 allows)? (In case it'd make sense to support these at > > some point?) Or is this over-engineering already? > > These are virtual frames. There are archs which have varying page size. I'd rather avoid introducing the page size, can't mig cope with 64bit values on 32bit archs? Samuel
Re: user-level drivers
Richard Braun, le Mon 09 May 2011 10:55:36 +0200, a écrit : > This RPC lacks a few additional constraints like boundaries, alignment > and maybe phase. What do you mean by "phase"? Samuel
Re: user-level drivers
On Mon, May 09, 2011 at 01:27:32PM +0200, Thomas Schwinge wrote: > > I'd suggest using natural_t (or unsigned long) too. But then, it can't > > be used to address >4 GiB physical memory. Consider expressing physical > > memory in page frame numbers. > > Good idea! But: what about differently sized frames (4 KiB/2 > MiB/whatever amd64 allows)? (In case it'd make sense to support these at > some point?) Or is this over-engineering already? These are virtual frames. And in any case, they are multiple of the base page size. This interface is suitable for allocating large pages. It would just require the requested size to be a multiple of the large page size to indicate large pages are wanted. This "large page" size would be provided through a new macro sitting next to PAGE_SIZE. -- Richard Braun
Re: user-level drivers
Hallo! On Mon, 9 May 2011 13:19:22 +0200, Richard Braun wrote: > On Mon, May 09, 2011 at 12:17:51PM +0200, Samuel Thibault wrote: > > > Hmm, I guess we don't have anything that is better than using > > > vm_address_t for physical addresses? At least not in > > > include/mach/std_types.h, i386/include/mach/i386/vm_types.h. Should we? > > > (phys_address_t based on natural_t?) > > > > Maybe we should, indeed, else we can't do PAE. > > I'd suggest using natural_t (or unsigned long) too. But then, it can't > be used to address >4 GiB physical memory. Consider expressing physical > memory in page frame numbers. Good idea! But: what about differently sized frames (4 KiB/2 MiB/whatever amd64 allows)? (In case it'd make sense to support these at some point?) Or is this over-engineering already? Grüße, Thomas pgpTtOACGU65b.pgp Description: PGP signature
Re: user-level drivers
Hallo! On Mon, 9 May 2011 12:17:51 +0200, Samuel Thibault wrote: > Thomas Schwinge, le Mon 09 May 2011 11:15:15 +0200, a écrit : > > > The patches however add a few > > > kernel RPCs, which we should probably agree on first, at the minimum > > > that their existence makes sense, so we can reserve slots in upstream > > > gnumach. Basically, it's about allocating physically-contiguous memory > > > for DMAs, and getting IRQ events: > > > > Of course, it doesn't matter at the moment, but are these for x86 only, > > or architecture independent? (I meant that given the non-usability of GNU Mach on other machines, it doesn't matter interface-wise whether we put them into mach.defs or mach_i386.defs.) Of course, making it independent of the architecture right now is better. > Well, it does matter since that changes the allocation which we'd do. I > believe that as it is proposed, it is architecture-agnostic: allocating > a physically-contiguous area of memory for things like DMAs, and being > notified of hardware requests. The vocabulary of the calls should be > made arch-neutral of course. Ack. > > Hmm, I guess we don't have anything that is better than using > > vm_address_t for physical addresses? At least not in > > include/mach/std_types.h, i386/include/mach/i386/vm_types.h. Should we? > > (phys_address_t based on natural_t?) > > Maybe we should, indeed, else we can't do PAE. But -- if this can return full PAE addresses -- then a natural_t isn't big enough either. As we have to be PAE-agnostic at this level (you can exchange a PAE with a non-PAE kernel, and expect your system still to work), we'd need something that works in either case -- in any cases where we mean to be ABI compatible, which for PAE va. non-PAE only is physical addresses. (Same for the Xen port, I guess?) While we're at this, we could also think about any issues with running a 32 bit user land on a 64 bit GNU Mach -- but at this kernel interface, that's already covered with PAE addresses, I think? > > At this time, would it make sense to make paddr inout (and/or vaddr, > > too?) and add an additional ``anywhere : boolean_t'' parameter as > > vm_allocate has? Or can't there be any need for such functionality? > > I was wondering too. I don't think it makes sense for paddr: either you > do care, and then should simply use /dev/mem and whatever we implement > behind (currently it's the mem driver), or you don't care. OK. > For vaddr, I was wondering. With Zheng Da's current implementation it's > not trivial to add !anywhere support, but it could make sense indeed. OK. We don't have to implement it (if we don't have a need for it at the moment), but we should provision for it in the RPC interface (and always have it fail with KERN_INVALID_ARGUMENT or whatever is appropriate). Grüße, Thomas pgpzwxt0UL0gt.pgp Description: PGP signature
Re: user-level drivers
On Mon, May 09, 2011 at 12:17:51PM +0200, Samuel Thibault wrote: > > Hmm, I guess we don't have anything that is better than using > > vm_address_t for physical addresses? At least not in > > include/mach/std_types.h, i386/include/mach/i386/vm_types.h. Should we? > > (phys_address_t based on natural_t?) > > Maybe we should, indeed, else we can't do PAE. I'd suggest using natural_t (or unsigned long) too. But then, it can't be used to address >4 GiB physical memory. Consider expressing physical memory in page frame numbers. -- Richard Braun
Re: user-level drivers
Thomas Schwinge, le Mon 09 May 2011 11:15:15 +0200, a écrit : > > The patches however add a few > > kernel RPCs, which we should probably agree on first, at the minimum > > that their existence makes sense, so we can reserve slots in upstream > > gnumach. Basically, it's about allocating physically-contiguous memory > > for DMAs, and getting IRQ events: > > Of course, it doesn't matter at the moment, but are these for x86 only, > or architecture independent? Well, it does matter since that changes the allocation which we'd do. I believe that as it is proposed, it is architecture-agnostic: allocating a physically-contiguous area of memory for things like DMAs, and being notified of hardware requests. The vocabulary of the calls should be made arch-neutral of course. > Hmm, I guess we don't have anything that is better than using > vm_address_t for physical addresses? At least not in > include/mach/std_types.h, i386/include/mach/i386/vm_types.h. Should we? > (phys_address_t based on natural_t?) Maybe we should, indeed, else we can't do PAE. > At this time, would it make sense to make paddr inout (and/or vaddr, > too?) and add an additional ``anywhere : boolean_t'' parameter as > vm_allocate has? Or can't there be any need for such functionality? I was wondering too. I don't think it makes sense for paddr: either you do care, and then should simply use /dev/mem and whatever we implement behind (currently it's the mem driver), or you don't care. For vaddr, I was wondering. With Zheng Da's current implementation it's not trivial to add !anywhere support, but it could make sense indeed. > > The actual event: > > > > simpleroutine mach_notify_irq( > >notify : notify_port_t; > >name: int); > > I don't understand this ``name : int''. Isn't this rather a pointer to a > struct mach_irq_notification_t? It's actually the interrupt line number. > What about using the I/O port scheme? That is, decide_intr_notify > doesn't enable IRQ notifications, but instead just returns a handle > (compare i386_io_perm_create) that is then passed to device_irq_enable to > enable/disable IRQ notifications (compare i386_io_perm_modify). Does > that make sense in this IRQ scenario? It probably looks better, yes. Samuel
Re: user-level drivers
Hallo! On Mon, 9 May 2011 00:07:16 +0200, Samuel Thibault wrote: > I've started having a look at Zheng Da's user-level driver integration. > I've cleaned his tree a bit, and now considering adding patches to > the debian packages for wider testing. Great! I'm not the most knowledgeable person to comment on these RPC interfaces, but anyway: > The patches however add a few > kernel RPCs, which we should probably agree on first, at the minimum > that their existence makes sense, so we can reserve slots in upstream > gnumach. Basically, it's about allocating physically-contiguous memory > for DMAs, and getting IRQ events: Of course, it doesn't matter at the moment, but are these for x86 only, or architecture independent? > * This routine is created for allocating DMA buffers. > * We are going to get a contiguous physical memory > * and its physical address in addition to the virtual address. > */ > routine vm_allocate_contiguous( > host_priv : host_priv_t; > target_task : vm_task_t; > out vaddr : vm_address_t; > out paddr : vm_address_t; > size: vm_size_t); Hmm, I guess we don't have anything that is better than using vm_address_t for physical addresses? At least not in include/mach/std_types.h, i386/include/mach/i386/vm_types.h. Should we? (phys_address_t based on natural_t?) At this time, would it make sense to make paddr inout (and/or vaddr, too?) and add an additional ``anywhere : boolean_t'' parameter as vm_allocate has? Or can't there be any need for such functionality? > /* Requesting IRQ events on a port */ > > routine device_intr_notify( >master_port : mach_port_t; >in irq : int; >in id : int; >in flags : int; >in receive_port: mach_port_send_t >); Minor, but I would put the receive_port at the top of the ``in data'' list, after the master_port. Shouldn't all the other ``in data'' items be wrapped in an (architecured-dependent) structure similar to what we're doing for I/O ports? Compare io_perm_t in i386/include/mach/i386/mach_i386.defs, i386/include/mach/i386/mach_i386_types.h, i386/i386/io_perm.h. I believe this would help to separate implementation details (IRQ number being an integer; specific values of flags; etc.) from the RPC mechanism. Especially so if these are architecture independent calls. > The actual event: > > simpleroutine mach_notify_irq( >notify : notify_port_t; >name: int); I don't understand this ``name : int''. Isn't this rather a pointer to a struct mach_irq_notification_t? > And a way to mask/unmask irqs: > > /* > * enable/disable the specified irq. > */ > routine device_irq_enable( >master_port : mach_port_t; >irq : int; >status : char); > > Should this be rather split into device_irq_enable/disable and drop the > status paramter? What about using the I/O port scheme? That is, decide_intr_notify doesn't enable IRQ notifications, but instead just returns a handle (compare i386_io_perm_create) that is then passed to device_irq_enable to enable/disable IRQ notifications (compare i386_io_perm_modify). Does that make sense in this IRQ scenario? Grüße, Thomas pgpKNGJJPeDT8.pgp Description: PGP signature
Re: user-level drivers
On Mon, May 09, 2011 at 12:07:16AM +0200, Samuel Thibault wrote: > I've started having a look at Zheng Da's user-level driver integration. > I've cleaned his tree a bit, and now considering adding patches to > the debian packages for wider testing. The patches however add a few > kernel RPCs, which we should probably agree on first, at the minimum > that their existence makes sense, so we can reserve slots in upstream > gnumach. Basically, it's about allocating physically-contiguous memory > for DMAs, and getting IRQ events: > > /* > * This routine is created for allocating DMA buffers. > * We are going to get a contiguous physical memory > * and its physical address in addition to the virtual address. > */ > routine vm_allocate_contiguous( > host_priv : host_priv_t; > target_task : vm_task_t; > out vaddr : vm_address_t; > out paddr : vm_address_t; > size: vm_size_t); This RPC lacks a few additional constraints like boundaries, alignment and maybe phase. We may not use them now, but they're important for portability (e.g. if GNU Mach supports PAE, drivers that can't use physical memory beyond the 4 GiB limit must be able to express it). > /* Requesting IRQ events on a port */ > > routine device_intr_notify( >master_port : mach_port_t; >in irq : int; >in id : int; >in flags : int; >in receive_port: mach_port_send_t >); > > The actual event: > > simpleroutine mach_notify_irq( >notify : notify_port_t; >name: int); > > And a way to mask/unmask irqs: > > /* > * enable/disable the specified irq. > */ > routine device_irq_enable( >master_port : mach_port_t; >irq : int; >status : char); > > Should this be rather split into device_irq_enable/disable and drop the > status paramter? Naming a function taht can disable something "xxx_enable" is confusing. By the way, I also think using both "intr" and "irq" is confusing. We should stick with "intr". device_intr_notify() isn't a suitable name as it doesn't make it obvious that it's a registration request. It should rather be named device_intr_establish() or device_intr_register(). Parameters named "irq" could be renamed "line" instead. And we should also follow the way Mach names are built (i.e. module_object_method). mach_notify_irq() could be renamed mach_intr_notify(). device_irq_enable() would be named device_intr_enable(). -- Richard Braun
user-level drivers
Hello, I've started having a look at Zheng Da's user-level driver integration. I've cleaned his tree a bit, and now considering adding patches to the debian packages for wider testing. The patches however add a few kernel RPCs, which we should probably agree on first, at the minimum that their existence makes sense, so we can reserve slots in upstream gnumach. Basically, it's about allocating physically-contiguous memory for DMAs, and getting IRQ events: /* * This routine is created for allocating DMA buffers. * We are going to get a contiguous physical memory * and its physical address in addition to the virtual address. */ routine vm_allocate_contiguous( host_priv : host_priv_t; target_task : vm_task_t; out vaddr : vm_address_t; out paddr : vm_address_t; size: vm_size_t); /* Requesting IRQ events on a port */ routine device_intr_notify( master_port : mach_port_t; in irq : int; in id : int; in flags : int; in receive_port: mach_port_send_t ); The actual event: simpleroutine mach_notify_irq( notify : notify_port_t; name: int); And a way to mask/unmask irqs: /* * enable/disable the specified irq. */ routine device_irq_enable( master_port : mach_port_t; irq : int; status : char); Should this be rather split into device_irq_enable/disable and drop the status paramter? Samuel
