[Qemu-devel] Re: [PATCH 3/3] msi: Store the capability size in PCIDevice

[Michael S. Tsirkin]

On Mon, Nov 01, 2010 at 11:37:53PM -0600, Alex Williamson wrote:
> Avoid needing to get the MSI capability flags every time we need to
> check the capability length.  This also makes it accessible outside
> of msi.c, making it easier for users to filter config space writes
> using msi_cap and msi_cap_size.

I think for this last use-case, we are better off with returning a
boolean from msi_write_config which tells us whether the write is in
range. This has the advantage in that it will also work well for other
capabilities. Or second best, if that is insufficient for some reason,
export an msi_cap_size function.

> 
> Signed-off-by: Alex Williamson <address@hidden>
> ---
> 
>  hw/msi.c |    9 ++++-----
>  hw/pci.h |    3 ++-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/msi.c b/hw/msi.c
> index 110859b..12e125f 100644
> --- a/hw/msi.c
> +++ b/hw/msi.c
> @@ -148,6 +148,7 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
>      }
>  
>      dev->msi_cap = config_offset;
> +    dev->msi_cap_size = cap_size;
>      dev->cap_present |= QEMU_PCI_CAP_MSI;
>  
>      pci_set_word(dev->config + msi_flags_off(dev), flags);
> @@ -170,14 +171,12 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
>  
>  void msi_uninit(struct PCIDevice *dev)
>  {
> -    uint8_t cap_size;
> -
>      if (!(dev->cap_present & QEMU_PCI_CAP_MSI))
>          return;
>  
> -    cap_size = msi_cap_sizeof(pci_get_word(dev->config + 
> msi_flags_off(dev)));
> -    pci_del_capability(dev, PCI_CAP_ID_MSIX, cap_size);
> +    pci_del_capability(dev, PCI_CAP_ID_MSIX, dev->msi_cap_size);
>      dev->msi_cap = 0;
> +    dev->msi_cap_size = 0;
>      dev->cap_present &= ~QEMU_PCI_CAP_MSI;
>      MSI_DEV_PRINTF(dev, "uninit\n");
>  }
> @@ -269,7 +268,7 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, 
> uint32_t val, int len)
>      uint32_t pending;
>      int i;
>  
> -    if (!ranges_overlap(addr, len, dev->msi_cap, msi_cap_sizeof(flags))) {
> +    if (!ranges_overlap(addr, len, dev->msi_cap, dev->msi_cap_size)) {
>          return;
>      }
>  
> diff --git a/hw/pci.h b/hw/pci.h
> index a558803..d268806 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -176,8 +176,9 @@ struct PCIDevice {
>      /* Version id needed for VMState */
>      int32_t version_id;
>  
> -    /* Offset of MSI capability in config space */
> +    /* Offset & size of MSI capability in config space */
>      uint8_t msi_cap;
> +    uint8_t msi_cap_size;
>  
>      /* PCI Express */
>      PCIExpressDevice exp;