* Michael S. Tsirkin (mst@xxxxxxxxxx) wrote:
> +struct generic_dev {
I know I commented on this one on an earlier, private version, and naming
is not my strength... maybe "struct uio_generic_pci_dev" or "struct
uio_generic_pci"?
> + struct uio_info info;
> + struct pci_dev *pdev;
> + spinlock_t lock; /* guards command register accesses */
> +};
> +
> +/* Read/modify/write command register to disable interrupts.
> + * Note: we could cache the value and optimize the read if there was a way to
> + * get notified of user changes to command register through sysfs.
> + * */
For the irqcontrol case, I don't think RMW is a problem (coming from
userspace it's already a slower path).
For the irqhandler case, you can grab the full dword to get Command+Status
(since you needed status anyway, and config reads are dword).
> +static void irqtoggle(struct generic_dev *dev, int irq_on)
> +{
> + struct pci_dev *pdev = dev->pdev;
> + unsigned long flags;
> + u16 orig, new;
> +
> + spin_lock_irqsave(&dev->lock, flags);
> + pci_block_user_cfg_access(pdev);
> + pci_read_config_word(pdev, PCI_COMMAND, &orig);
> + new = irq_on ? orig & ~PCI_COMMAND_INTX_DISABLE :
> + orig | PCI_COMMAND_INTX_DISABLE;
> + if (new != orig)
> + pci_write_config_word(dev->pdev, PCI_COMMAND, new);
> + pci_unblock_user_cfg_access(dev);
> + spin_unlock_irqrestore(&dev->lock, flags);
> +}
> +
> +/* irqcontrol is use by userspace to enable/disable interrupts. */
> +static int irqcontrol(struct uio_info *info, s32 irq_on)
> +{
> + struct generic_dev *dev = container_of(info, struct generic_dev, info);
> + irqtoggle(dev, irq_on);
> + return 0;
> +}
> +
> +static irqreturn_t irqhandler(int irq, struct uio_info *info)
> +{
> + struct generic_dev *dev = container_of(info, struct generic_dev, info);
> + irqreturn_t ret = IRQ_NONE;
> + u16 status;
> +
> + /* Check interrupt status register to see whether our device
> + * triggered the interrupt. */
> + pci_read_config_word(dev->pdev, PCI_STATUS, &status);
> + if (!(status & PCI_STATUS_INTERRUPT))
> + goto done;
> +
> + /* We triggered the interrupt, disable it. */
> + irqtoggle(dev, 0);
> + /* UIO core will signal the user process. */
> + ret = IRQ_HANDLED;
> +done:
> + return ret;
> +}
> +
> +/* Verify that the device supports Interrupt Disable bit in command register,
> + * per PCI 2.3, by flipping this bit and reading it back: this bit was readonly
> + * in PCI 2.2. */
Wonder if this should also restrict from things like bridges?
> +static int __devinit verify_pci_2_3(struct pci_dev *pdev)
> +{
> + u16 orig, new;
> + int err = 0;
> +
> + pci_block_user_cfg_access(pdev);
> + pci_read_config_word(pdev, PCI_COMMAND, &orig);
> + pci_write_config_word(pdev, PCI_COMMAND,
> + orig ^ PCI_COMMAND_INTX_DISABLE);
> + pci_read_config_word(pdev, PCI_COMMAND, &new);
> + /* There's no way to protect against
> + * hardware bugs or detect them reliably, but as long as we know
> + * what the value should be, let's go ahead and check it. */
> + if ((new ^ orig) & ~PCI_COMMAND_INTX_DISABLE) {
> + err = -EBUSY;
> + dev_err(&pdev->dev, "Command changed from 0x%x to 0x%x: "
> + "driver or HW bug?\n", orig, new);
> + goto err;
> + }
> + if (!((new ^ orig) & PCI_COMMAND_INTX_DISABLE)) {
> + dev_warn(&pdev->dev, "Device does not support "
> + "disabling interrupts: unable to bind.\n");
> + err = -ENODEV;
> + goto err;
> + }
> + /* Now restore the original value. */
> + pci_write_config_word(pdev, PCI_COMMAND, orig);
> +err:
> + pci_unblock_user_cfg_access(pdev);
> + return err;
> +}
> +
> +static int __devinit probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + struct generic_dev *dev;
> + int err;
> +
> + err = pci_enable_device(pdev);
> + if (err) {
> + dev_err(&pdev->dev, "%s: pci_enable_device failed: %d\n",
> + __func__, err);
> + return err;
> + }
> +
> + err = verify_pci_2_3(pdev);
> + if (err)
> + goto err_verify;
> +
> + err = pci_request_regions(pdev, "uio_pci_generic");
> + if (err) {
> + dev_err(&pdev->dev, "%s: pci_request_regions failed: %d\n",
> + __func__, err);
> + goto err_verify;
> + }
> +
> + dev = kzalloc(sizeof(struct generic_dev), GFP_KERNEL);
> + if (!dev) {
> + err = -ENOMEM;
> + goto err_alloc;
> + }
> +
> + dev->info.name = "uio_pci_generic";
> + dev->info.version = "0.01";
> + dev->info.irq = pdev->irq;
May need to verify pdev->irq in verify too?
> + dev->info.irq_flags = IRQF_SHARED;
> + dev->info.handler = irqhandler;
> + dev->info.irqcontrol = irqcontrol;
> + dev->pdev = pdev;
> + spin_lock_init(&dev->lock);
> +
> + pci_reset_function(pdev);
I think this could be a bit much, since it will fall back to secondary
bus reset.
thanks,
-chris
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
