[Qemu-devel] Re: [PATCH v9 2/8] pci: fix accesses to pci status register

[Michael S. Tsirkin]

On Tue, Nov 16, 2010 at 05:26:06PM +0900, Isaku Yamahata wrote:
> pci status register is 16 bit, not 8 bit.
> So use helper function to manipulate status register.
> 
> Signed-off-by: Isaku Yamahata <address@hidden>

At least the subject is wrong: the relevant bit is in the low byte. So
the code is correct as written on BE machines.

> ---
>  hw/pci.c |   21 +++++++++++++--------
>  1 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 2fc8ab1..52fe655 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -127,9 +127,11 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int 
> irq_num, int change)
>  static void pci_update_irq_status(PCIDevice *dev)
>  {
>      if (dev->irq_state) {
> -        dev->config[PCI_STATUS] |= PCI_STATUS_INTERRUPT;
> +        pci_word_test_and_set_mask(dev->config + PCI_STATUS,
> +                                   PCI_STATUS_INTERRUPT);
>      } else {
> -        dev->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> +        pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
> +                                     PCI_STATUS_INTERRUPT);
>      }
>  }
>  
> @@ -404,7 +406,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
>       * in irq_state which we are saving.
>       * This makes us compatible with old devices
>       * which never set or clear this bit. */
> -    s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> +    pci_word_test_and_clear_mask(s->config + PCI_STATUS, 
> PCI_STATUS_INTERRUPT);
>      vmstate_save_state(f, pci_get_vmstate(s), s);
>      /* Restore the interrupt status bit. */
>      pci_update_irq_status(s);
> @@ -530,7 +532,7 @@ static void pci_init_cmask(PCIDevice *dev)
>  {
>      pci_set_word(dev->cmask + PCI_VENDOR_ID, 0xffff);
>      pci_set_word(dev->cmask + PCI_DEVICE_ID, 0xffff);
> -    dev->cmask[PCI_STATUS] = PCI_STATUS_CAP_LIST;
> +    pci_set_word(dev->cmask + PCI_STATUS, PCI_STATUS_CAP_LIST);
>      dev->cmask[PCI_REVISION_ID] = 0xff;
>      dev->cmask[PCI_CLASS_PROG] = 0xff;
>      pci_set_word(dev->cmask + PCI_CLASS_DEVICE, 0xffff);
> @@ -1697,8 +1699,9 @@ static uint8_t pci_find_capability_list(PCIDevice 
> *pdev, uint8_t cap_id,
>  {
>      uint8_t next, prev;
>  
> -    if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST))
> +    if (!(pci_get_word(pdev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST)) {
>          return 0;
> +    }
>  
>      for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]);
>           prev = next + PCI_CAP_LIST_NEXT)
> @@ -1804,7 +1807,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
>      config[PCI_CAP_LIST_ID] = cap_id;
>      config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
>      pdev->config[PCI_CAPABILITY_LIST] = offset;
> -    pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
> +    pci_word_test_and_set_mask(pdev->config + PCI_STATUS, 
> PCI_STATUS_CAP_LIST);
>      memset(pdev->used + offset, 0xFF, size);
>      /* Make capability read-only by default */
>      memset(pdev->wmask + offset, 0, size);
> @@ -1827,8 +1830,10 @@ void pci_del_capability(PCIDevice *pdev, uint8_t 
> cap_id, uint8_t size)
>      memset(pdev->cmask + offset, 0, size);
>      memset(pdev->used + offset, 0, size);
>  
> -    if (!pdev->config[PCI_CAPABILITY_LIST])
> -        pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
> +    if (!pdev->config[PCI_CAPABILITY_LIST]) {
> +        pci_word_test_and_clear_mask(pdev->config + PCI_STATUS,
> +                                     PCI_STATUS_CAP_LIST);
> +    }
>  }
>  
>  /* Reserve space for capability at a known offset (to call after load). */
> -- 
> 1.7.1.1