Message ID | 20101027140558.GA3380@redhat.com |
---|---|
State | New |
Headers | show |
On Wed, Oct 27, 2010 at 04:05:58PM +0200, Michael S. Tsirkin wrote: > - save/restore must not check w1c bits > since they are in fact guest controlled > - clear w1c bits on reset > > Note: for express there are different kinds of > reset, some leave part of config space alone. > We will likely need a sticky bit mask to implement this. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > Isaku, does the below make sense? > > hw/pci.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index e50de14..1a14a0e 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -141,7 +141,8 @@ static void pci_device_reset(PCIDevice *dev) > pci_update_irq_status(dev); > /* Clear all writeable bits */ > pci_word_test_and_clear_mask(dev->config + PCI_COMMAND, > - pci_get_word(dev->wmask + PCI_COMMAND)); > + pci_get_word(dev->wmask + PCI_COMMAND) | > + pci_get_word(dev->w1cmask + PCI_COMMAND)); > dev->config[PCI_CACHE_LINE_SIZE] = 0x0; > dev->config[PCI_INTERRUPT_LINE] = 0x0; > for (r = 0; r < PCI_NUM_REGIONS; ++r) { > @@ -293,7 +294,8 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size) > > qemu_get_buffer(f, config, size); > for (i = 0; i < size; ++i) { > - if ((config[i] ^ s->config[i]) & s->cmask[i] & ~s->wmask[i]) { > + if ((config[i] ^ s->config[i]) & > + s->cmask[i] & ~s->wmask[i] & ~s->w1cmask) { [i] is needed for w1cmask. ~s->w1cmask[i] ^^^ > qemu_free(config); > return -EINVAL; > } > -- > 1.7.3.2.91.g446ac >
On Thu, Oct 28, 2010 at 11:02:48AM +0900, Isaku Yamahata wrote: > On Wed, Oct 27, 2010 at 04:05:58PM +0200, Michael S. Tsirkin wrote: > > - save/restore must not check w1c bits > > since they are in fact guest controlled > > - clear w1c bits on reset > > > > Note: for express there are different kinds of > > reset, some leave part of config space alone. > > We will likely need a sticky bit mask to implement this. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > > > Isaku, does the below make sense? > > > > hw/pci.c | 6 ++++-- > > 1 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/hw/pci.c b/hw/pci.c > > index e50de14..1a14a0e 100644 > > --- a/hw/pci.c > > +++ b/hw/pci.c > > @@ -141,7 +141,8 @@ static void pci_device_reset(PCIDevice *dev) > > pci_update_irq_status(dev); > > /* Clear all writeable bits */ > > pci_word_test_and_clear_mask(dev->config + PCI_COMMAND, > > - pci_get_word(dev->wmask + PCI_COMMAND)); > > + pci_get_word(dev->wmask + PCI_COMMAND) | > > + pci_get_word(dev->w1cmask + PCI_COMMAND)); > > dev->config[PCI_CACHE_LINE_SIZE] = 0x0; > > dev->config[PCI_INTERRUPT_LINE] = 0x0; > > for (r = 0; r < PCI_NUM_REGIONS; ++r) { > > @@ -293,7 +294,8 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size) > > > > qemu_get_buffer(f, config, size); > > for (i = 0; i < size; ++i) { > > - if ((config[i] ^ s->config[i]) & s->cmask[i] & ~s->wmask[i]) { > > + if ((config[i] ^ s->config[i]) & > > + s->cmask[i] & ~s->wmask[i] & ~s->w1cmask) { > > [i] is needed for w1cmask. > ~s->w1cmask[i] > ^^^ Yes, I sent out a borken patch but it's correct on my branch. > > > > qemu_free(config); > > return -EINVAL; > > } > > -- > > 1.7.3.2.91.g446ac > > > > -- > yamahata
diff --git a/hw/pci.c b/hw/pci.c index e50de14..1a14a0e 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -141,7 +141,8 @@ static void pci_device_reset(PCIDevice *dev) pci_update_irq_status(dev); /* Clear all writeable bits */ pci_word_test_and_clear_mask(dev->config + PCI_COMMAND, - pci_get_word(dev->wmask + PCI_COMMAND)); + pci_get_word(dev->wmask + PCI_COMMAND) | + pci_get_word(dev->w1cmask + PCI_COMMAND)); dev->config[PCI_CACHE_LINE_SIZE] = 0x0; dev->config[PCI_INTERRUPT_LINE] = 0x0; for (r = 0; r < PCI_NUM_REGIONS; ++r) { @@ -293,7 +294,8 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size) qemu_get_buffer(f, config, size); for (i = 0; i < size; ++i) { - if ((config[i] ^ s->config[i]) & s->cmask[i] & ~s->wmask[i]) { + if ((config[i] ^ s->config[i]) & + s->cmask[i] & ~s->wmask[i] & ~s->w1cmask) { qemu_free(config); return -EINVAL; }
- save/restore must not check w1c bits since they are in fact guest controlled - clear w1c bits on reset Note: for express there are different kinds of reset, some leave part of config space alone. We will likely need a sticky bit mask to implement this. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- Isaku, does the below make sense? hw/pci.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)