On Mon, Nov 07, 2016 at 06:59:30PM +0100, Andrew Jones wrote: > On Tue, Nov 08, 2016 at 01:35:02AM +0800, Peter Xu wrote: > > On Fri, Nov 04, 2016 at 06:04:56PM +0100, Andrew Jones wrote: > > > On Wed, Oct 26, 2016 at 03:47:13PM +0800, Peter Xu wrote: > > > > We need to call this whenever we want to do DMA with a device. > > > > > > > > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx> > > > > --- > > > > lib/pci.c | 7 +++++++ > > > > lib/pci.h | 1 + > > > > 2 files changed, 8 insertions(+) > > > > > > > > diff --git a/lib/pci.c b/lib/pci.c > > > > index 95a6581..1495f61 100644 > > > > --- a/lib/pci.c > > > > +++ b/lib/pci.c > > > > @@ -7,6 +7,13 @@ > > > > #include "pci.h" > > > > #include "asm/pci.h" > > > > > > > > +void pci_set_master(struct pci_dev *dev, int master) > > > > > > Why take pci_dev instead of just the devid/bdf here? > > > What is the 'master' argument (which is unused and at least > > > needs an __unused to compile) here for? > > > > I think both works for me here, but when this happens, IMHO pci_dev > > struct is prefered, since as long as the caller has one *dev pointer > > (I think if we introduce pci_dev, people should always keep one object > > for each PCI device they manipulate in kvm-unit-tests), I would just > > let people to pass in "dev" rather than "dev->pci_bdf", since it's > > shorter for typing, and also it avoids referencing internal fields. > > > > Regarding to the "master" variable... Oops, I just missed that. What I > > want to do is: > > > > if (master) > > val |= PCI_COMMAND_MASTER; > > else > > val &= ~PCI_COMMAND_MASTER; > > > > Thanks for pointing out. Will fix. > > > Actually, maybe we can just drop this patch. It's just > > pci_config_writew(dev->bsd, PCI_COMMAND, > pci_config_readw(dev->bsd, PCI_COMMAND) | PCI_COMMAND_MASTER); > ^ change op as needed > > Long, but no big deal... If updating PCI_COMMAND is a common thing > to do then we should have > > pci_command_set(dev, bits) > pci_command_clr(dev, bits) > > operators instead. Sounds good to me. Actually I think it does not hurt to provide a pci_set_master(), even as a wrapper to pci_command_set(). But for now, let me drop pci_set_master() in this series. Maybe I can use one function pci_cmd_set_clr(dev, set_bits, clr_bits) for the two? Thanks, -- peterx -- 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
