On Thu, Oct 20, 2016 at 02:49:50PM +0200, Andrew Jones wrote:
> On Fri, Oct 14, 2016 at 08:40:47PM +0800, Peter Xu wrote:
> > And set master by default for pci devices. So that DMA is allowed.
> >
> > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> > ---
> > lib/pci.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/lib/pci.c b/lib/pci.c
> > index ef0b02a..044e4db 100644
> > --- a/lib/pci.c
> > +++ b/lib/pci.c
> > @@ -7,6 +7,13 @@
> > #include "pci.h"
> > #include "asm/pci.h"
> >
> > +static void pci_set_master(pci_dev *dev, int master)
> > +{
> > + uint32_t val = pci_config_read(dev->pci_addr, PCI_COMMAND);
> > + val |= PCI_COMMAND_MASTER;
> > + pci_config_write(dev->pci_addr, PCI_COMMAND, val);
> > +}
>
> I agree with the API extension (well, I know next to nothing about PCI,
> so I'll assume this function is correct...) But...
>
> > +
> > /*
> > * Scan bus look for a specific device. Only bus 0 scanned for now.
> > * After the scan, a pci_dev is returned with correct BAR information.
> > @@ -41,6 +48,8 @@ int pci_dev_init(pci_dev *dev, uint16_t vendor_id, uint16_t device_id)
> > }
> > dev->inited = true;
> >
> > + pci_set_master(dev, 1);
>
> ...I don't necessarily agree with it being enabled by default. In the
> least I think you want a minimal init function available to unit tests
> that they then can extend with their own custom init. Then you can also
> provide an init that configures a bunch of stuff you think is a
> good idea by default. For example, in my gic series I try to do that. I
> have a gic_init() function which does nothing except set some base
> addresses. Then, I have a gic_enable_defaults function that unit tests
> can use if they don't care how we enable it, they just want something
> that works. Otherwise the unit test would just call gic_init and then
> write all the registers for enabling it that they want/need.
Good idea. will follow.
First of all, I'll introduce pci_dev_init() to init the raw structure.
Then, will add one pci_enable_defaults() with all the rests.
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
