On Thu, 12 Nov 2009 15:51:29 +0800 "Cui, Dexuan" <dexuan.cui@xxxxxxxxx> wrote: > Hi Jesse, > May I also have your comment? > > Thanks, > -- Dexuan > > > -----Original Message----- > From: linux-pci-owner@xxxxxxxxxxxxxxx > [mailto:linux-pci-owner@xxxxxxxxxxxxxxx] On Behalf Of Cui, Dexuan > Sent: Tuesday, November 10, 2009 6:15 PM To: Matthew Wilcox > Cc: linux-pci@xxxxxxxxxxxxxxx > Subject: RE: [PATCH 1/3] PCI: support device-specific reset methods. > > Matthew Wilcox wrote: > > On Fri, Nov 06, 2009 at 05:24:55PM +0800, Dexuan Cui wrote: > >> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > >> index d92d195..02247ac 100644 > >> --- a/drivers/pci/pci.h > >> +++ b/drivers/pci/pci.h > >> @@ -311,4 +311,12 @@ static inline int > >> pci_resource_alignment(struct pci_dev *dev, return > >> resource_alignment(res); } > >> > >> +struct pci_dev_reset_methods { > >> + u16 vendor; > >> + u16 device; > >> + int (*reset)(struct pci_dev *dev, int probe); > >> +}; > >> + > >> +extern struct pci_dev_reset_methods pci_dev_reset_methods[]; + > >> #endif /* DRIVERS_PCI_H */ > > > > Why do it this way instead of having a ->reset method in struct > > pci_driver? > Sorry for the late reply as I was on an accidental leave. > > Hi Matthew, > Currently pci_reset_function() is mainly used by > kvm_vm_ioctl_assign_device() and kvm_free_assigned_device(). In the > case of KVM VT-d, in host the driver of the device assigned to HVM > guest is not the "genuine" one -- it should always be the dummy > driver "pci-stub" or NULL, because the "genuine" driver in HVM guest > would control the device and in host we should not load an actual > driver for the device. > > So adding a 'reset' field into struct pci_driver doesn't sound like a > good idea to me. And the existing pci_dev_reset() has been there for > quite a while, so I think adding pci_dev_specific_reset() into > pci_dev_reset() should be good. Moving things to a ->reset member function seems like a good cleanup. Drivers that need specific reset funcs could override it and call the default one as necessary from their private functions. Can you take a look at that and see if it would be much harder than the scheme you proposed? I think it would end up a bit cleaner, and shouldn't be much more code... Thanks, -- Jesse Barnes, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html