Re: Broken pci_block_user_cfg_access interface

[Brian King <brking@linux.vnet.ibm.com>]

On 08/24/2011 05:43 AM, Jan Kiszka wrote:
> Hi,
> 
> trying to port the generic device interrupt masking pattern of
> uio_pci_generic to KVM's device assignment code, I stumbled over some
> fundamental problem with the current pci_block/unblock_user_cfg_access
> interface: it does not provide any synchronization between blocking
> sides. This allows user space to trigger a kernel BUG, just run two
> 
> while true; do echo 1 > /sys/bus/pci/devices/<some-device>/reset; done
> 
> loops in parallel and watch the kernel oops.
> 
> Instead of some funky open-coded locking mechanism, we would rather need
> a plain mutex across both the user space access (via sysfs) and the
> sections guarded by pci_block/unblock_user_cfg_access so far. But I'm
> not sure which of them already allow sleeping, specifically if the IPR
> driver would be fine with such a change. Can someone in the CC list
> comment on this?

The ipr driver calls pci_block/unblock_user_cfg_access from interrupt
context, so a mutex won't work. When the pci_block/unblock API was
originally added, it did not have the checking it has today to detect
if it is being called nested. This was added some time later. The
API that works best for the ipr driver is to allow for many block calls,
but a single unblock call unblocks access. It seems like what might
work well in the case above is a block count. Each call to pci_block
increments a count. Each pci_unblock decrements the count and only
actually do the unblock if the count drops to zero. It should be reasonably
simple for ipr to use that sort of an API as well.

-Brian

-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center