[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabi
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities |
Date: |
Wed, 10 Jun 2009 18:01:29 +0300 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
On Wed, Jun 10, 2009 at 11:55:40AM -0300, Glauber Costa wrote:
> On Wed, Jun 10, 2009 at 12:54:15PM +0300, Michael S. Tsirkin wrote:
> > On Tue, Jun 09, 2009 at 02:11:14PM -0300, Glauber Costa wrote:
> > > On Fri, Jun 05, 2009 at 01:23:15PM +0300, Michael S. Tsirkin wrote:
> > > > Add routines to manage PCI capability list. First user will be MSI-X.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > > > ---
> > > > hw/pci.c | 98
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
> > > > hw/pci.h | 18 +++++++++++-
> > > > 2 files changed, 106 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/hw/pci.c b/hw/pci.c
> > > > index 361d741..ed011b5 100644
> > > > --- a/hw/pci.c
> > > > +++ b/hw/pci.c
> > > > @@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > > > int version = s->cap_present ? 3 : 2;
> > > > int i;
> > > >
> > > > - qemu_put_be32(f, version); /* PCI device version */
> > > > + /* PCI device version and capabilities */
> > > > + qemu_put_be32(f, version);
> > > > + if (version >= 3)
> > > > + qemu_put_be32(f, s->cap_present);
> > > > qemu_put_buffer(f, s->config, 256);
> > > > for (i = 0; i < 4; i++)
> > > > qemu_put_be32(f, s->irq_state[i]);
> > > > - if (version >= 3)
> > > > - qemu_put_be32(f, s->cap_present);
> > > > }
> > > What is it doing here?
> > > You should just do it right in the first patch, instead of doing in
> > > one way there, and fixing here.
> > >
> > > >
> > > > int pci_device_load(PCIDevice *s, QEMUFile *f)
> > > > @@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)
> > > > version_id = qemu_get_be32(f);
> > > > if (version_id > 3)
> > > > return -EINVAL;
> > > > - qemu_get_buffer(f, s->config, 256);
> > > > - pci_update_mappings(s);
> > > > -
> > > > - if (version_id >= 2)
> > > > - for (i = 0; i < 4; i ++)
> > > > - s->irq_state[i] = qemu_get_be32(f);
> > > > if (version_id >= 3)
> > > > s->cap_present = qemu_get_be32(f);
> > > > else
> > > ditto.
> > > > @@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f)
> > > > if (s->cap_present & ~s->cap_supported)
> > > > return -EINVAL;
> > > >
> > > > + qemu_get_buffer(f, s->config, 256);
> > > > + pci_update_mappings(s);
> > > > +
> > > > + if (version_id >= 2)
> > > > + for (i = 0; i < 4; i ++)
> > > > + s->irq_state[i] = qemu_get_be32(f);
> > > > + /* Clear wmask and used bits for capabilities.
> > > > + Must be restored separately, since capabilities can
> > > > + be placed anywhere in config space. */
> > > > + memset(s->used, 0, PCI_CONFIG_SPACE_SIZE);
> > > > + for (i = PCI_CONFIG_HEADER_SIZE; i < PCI_CONFIG_SPACE_SIZE; ++i)
> > > > + s->wmask[i] = 0xff;
> > > > return 0;
> > > > }
> > > Sorry, I don't exactly understand it. Although it can be anywhere, what
> > > do we actually
> > > lose by keeping it at the same place in config space?
> >
> > We lose the ability to let user control the capabilities exposed
> > by the device.
> >
> > And generally, I dislike arbitrary limitations. The PCI spec says the
> > capability can be anywhere, implementing a linked list of caps is simple
> > enough to not invent abritrary restrictions.
> yes, but this is migration time, right?
I think so, yes.
>
> caps can be anywhere, but we don't expect it to change during machine
> execution
> lifetime.
>
> Or I am just confused by the name "pci_device_load" ?
Right. So I want to load an image and it has capability X at offset Y.
wmask has to match. I don't want to assume that we never change Y
for the device without breaking old images, so I clear wmask here
and set it up again after looking up capabilities that I loaded.
Maybe this explanation should go into the comment above?
--
MST
- [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Michael S. Tsirkin, 2009/06/06
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Glauber Costa, 2009/06/09
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Michael S. Tsirkin, 2009/06/10
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Glauber Costa, 2009/06/10
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Paul Brook, 2009/06/10
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Michael S. Tsirkin, 2009/06/10
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Jamie Lokier, 2009/06/10
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Michael S. Tsirkin, 2009/06/10
- Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities, Jamie Lokier, 2009/06/10
- Configuration vs. compat hints [was Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities], Mark McLoughlin, 2009/06/12
- Re: Configuration vs. compat hints [was Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities], Michael S. Tsirkin, 2009/06/12
- Re: Configuration vs. compat hints [was Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities], Mark McLoughlin, 2009/06/12
- Re: Configuration vs. compat hints [was Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities], Anthony Liguori, 2009/06/12
- Re: Configuration vs. compat hints [was Re: [Qemu-devel] [PATCHv3 03/13] qemu: add routines to manage PCI capabilities], Mark McLoughlin, 2009/06/12