linux-kernel - Re: [PATCH 00/03][RFC] Reusable UIO Platform Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aec7e5c30805210109p664fd917r32f8dc6aa1c4c746@mail.gmail.com>
Date:	Wed, 21 May 2008 17:09:51 +0900
From:	"Magnus Damm" <magnus.damm@...il.com>
To:	"Uwe Kleine-König" <Uwe.Kleine-Koenig@...i.com>
Cc:	"Hans J. Koch" <hjk@...utronix.de>, linux-kernel@...r.kernel.org,
	lethal@...ux-sh.org, gregkh@...e.de, linux-sh@...r.kernel.org
Subject: Re: [PATCH 00/03][RFC] Reusable UIO Platform Driver

Hi Uwe!

Thanks for your email.

On Wed, May 21, 2008 at 3:49 PM, Uwe Kleine-König
<Uwe.Kleine-Koenig@...i.com> wrote:
> Magnus Damm wrote:
>> The uio_pdrv driver doesn't do what I need at this point, though I may
>> be able to extend it with the following:
>> - Interrupt enable/disable code
>> - Physically contiguous memory support
>>
>> The interrupt code may be placed in the board/cpu code, but I need to
>> share that code between multiple UIO driver instances. We want to use
>> the same UIO driver for many different processor models and hardware
>> blocks.
> What about adding uio_platform_handler (with a different name) to
> uio_pdrv.c?

I'm not sure if it will help. What would such function do? Please explain.

> OTOH I don't see why you want to disable the irq.  Can you describe the
> reason?

Most UIO kernel drivers today contain hardware device specific code to
acknowledge interrupts. In other words, most UIO interrupt handlers
touches some device specific bits so the interrupt gets deasserted.

My uio_platform driver handles interrupts in a different way. The
kernel UIO driver is not aware of the hardware device specific method
to acknowledge the interrupt, instead it simply disables the interrupt
and notifies user space which instead will acknowledge the interrupt.
Next time a read() or poll() call gets made, the interrupt is enabled
again.

This allows us to export a hardware device to user space and allow
user space to handle interrupts without knowing in kernel space how to
acknowledge interrupts.

>>         Extending uio_pdrv driver with a chunk of physically
>> contiguous memory isn't a big deal though.
> I wonder how you use that memory.  Isn't it just some kind of shared
> memory?  If so, why not use normal shared memory?  Do you really need
> that?

Yes, I need that to give the exported hardware device some physically
contiguous memory for DMA. At this point our hardware is missing IOMMU
so physically contiguous memory is needed.

>> To be frank, I have my doubts in adding an extra forwarding-only
>> platform layer on top of UIO compared to using uio_register_device()
>> directly from the board code. I like that the platform layer is using
>> struct resource and handles resource ranges for us automatically, but
>> wouldn't it make more sense to extend the UIO core to always use
>> struct resource instead of struct uio_mem? I'd be happy to help out -
>> just point me in the right direction.
> That alone doesn't help.  You need a struct device to register a uio
> device.  So a platform device is the straight forward approach.

I don't mind that you are using platform devices. Actually, I think
platform devices are great. We use them for all sorts of things on the
SuperH architecture. I'm trying to suggest that maybe it's a good idea
to change the UIO core code to use struct resource instead of struct
uio_mem. Or maybe that's not a good idea, I'm not sure.

>> >> The interrupt handling code in uio_platform assumes the device is the
>> >> only user of the assigned interrupt.
>> >
>> > Uwe's approach doesn't have this limitation.
>>
>> True, but the uio_pdrv driver is choosing to not deal with interrupts
>> at all. I'd like to have shared interrupt handling code. With my
>> driver, you just feed it io memory window parameters and an interrupt
>> number and off you go. No need for any callbacks.
> In my eyes this isn't completly correct.  Just the way you specify your
> handler is a bit different.  You can pass a handler via platform data
> with my driver, too.

I don't want to pass any handler. All devices share the same interrupt
handler, the only thing that differs between multiple uio_platform
devices on one system is memory window information and irq number.

> BTW, you don't need "depends on UIO" (because it's in a if UIO/endif
> block) and "default n" (as this is the default anyhow).  See also
> http://thread.gmane.org/gmane.linux.kernel/663884/focus=683097

Ah, thanks for pointing that out!

Thank you for your feedback!

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/