From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Fitzhardinge Subject: Re: [PATCH 2/6] Provide a mechanism to register domain owner of a PCI device. Date: Wed, 09 Dec 2009 13:11:30 -0800 Message-ID: <4B201282.3000609@goop.org> References: <1260391901-16685-1-git-send-email-konrad.wilk@oracle.com> <1260391901-16685-2-git-send-email-konrad.wilk@oracle.com> <1260391901-16685-3-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1260391901-16685-3-git-send-email-konrad.wilk@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 12/09/09 12:51, Konrad Rzeszutek Wilk wrote: > . and also to find the domain owner based on the PCI device and > to unregister a domain owner of a PCI device. > > Signed-off-by: Konrad Rzeszutek Wilk > --- > arch/x86/include/asm/xen/pci.h | 16 +++++++++ > arch/x86/xen/pci.c | 73 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 89 insertions(+), 0 deletions(-) > > diff --git a/arch/x86/include/asm/xen/pci.h b/arch/x86/include/asm/xen/pci.h > index cb84abe..f4312d5 100644 > --- a/arch/x86/include/asm/xen/pci.h > +++ b/arch/x86/include/asm/xen/pci.h > @@ -7,6 +7,11 @@ int xen_create_msi_irq(struct pci_dev *dev, > struct msi_desc *msidesc, > int type); > int xen_destroy_irq(int irq); > + > +int find_device_owner(struct pci_dev *dev); > +int register_device_owner(struct pci_dev *dev, domid_t domain); > +int unregister_device_owner(struct pci_dev *dev); > Could you give these less generic names? > + > #else > static inline int xen_register_gsi(u32 gsi, int triggering, int polarity) > { > @@ -23,6 +28,17 @@ static inline int xen_destroy_irq(int irq) > { > return -1; > } > + > +static inline int find_device_owner(struct pci_dev *dev) { return -1; } > +static inline int register_device_owner(struct pci_dev *dev, domid_t domain) > +{ > + return -1; > +} > +static inline int unregister_device_owner(struct pci_dev *dev) > +{ > + return -1; > +} > Make the formatting consistent here. > + > #endif > > #if defined(CONFIG_PCI_MSI)&& defined(CONFIG_XEN_DOM0_PCI) > diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c > index 44d91ad..4fa734c 100644 > --- a/arch/x86/xen/pci.c > +++ b/arch/x86/xen/pci.c > @@ -2,6 +2,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -109,3 +110,75 @@ error: > return ret; > } > #endif > + > +struct device_owner { > Again, a bit generic (the name should at least convey we're talking about owner domains). > + domid_t domain; > + struct pci_dev *dev; > + struct list_head list; > +}; > + > +static DEFINE_SPINLOCK(dev_domain_list_spinlock); > +static struct list_head dev_domain_list = LIST_HEAD_INIT(dev_domain_list); > + > +struct device_owner *find_device(struct pci_dev *dev) > +{ > + > + struct device_owner *owner; > Stray blank line. > + > + list_for_each_entry(owner,&dev_domain_list, list) { > + if (owner->dev == dev) > + return owner; > + } > + return NULL; > +} > + > +int find_device_owner(struct pci_dev *dev) > +{ > + > + struct device_owner *owner = NULL; > Again. > + > + owner = find_device(dev); > + if (!owner) > + return -ENODEV; > + > + return owner->domain; > +} > +EXPORT_SYMBOL(find_device_owner); > + > +int register_device_owner(struct pci_dev *dev, domid_t domain) > +{ > + struct device_owner *owner; > + unsigned long flags; > + > + if (find_device(dev)) > Should this be under the lock? > + return -EEXIST; > + > + owner = kzalloc(sizeof(struct device_owner), GFP_KERNEL); > Check for NULL. > + owner->domain = domain; > + owner->dev = dev; > + > + spin_lock_irqsave(&dev_domain_list_spinlock, flags); > + list_add_tail(&owner->list,&dev_domain_list); > + spin_unlock_irqrestore(&dev_domain_list_spinlock, flags); > + > + return 0; > +} > +EXPORT_SYMBOL(register_device_owner); > + > +int unregister_device_owner(struct pci_dev *dev) > +{ > + struct device_owner *owner = NULL; > + unsigned long flags; > + > + owner = find_device(dev); > Shouldn't this be under the lock too? > + if (!owner) > + return -ENODEV; > + > + spin_lock_irqsave(&dev_domain_list_spinlock, flags); > + list_del(&owner->list); > + spin_unlock_irqrestore(&dev_domain_list_spinlock, flags); > + > + kfree(owner); > + return 0; > +} > +EXPORT_SYMBOL(unregister_device_owner); > Thanks, J