[Qemu-devel] [PATCH] msix: Drop unused msix_bar_size

[Qemu-devel] [PATCH] msix: Drop unused msix_bar_size

[Jan Kiszka]

No user in sight.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/msix.c |    8 --------
 hw/msix.h |    2 --
 hw/pci.h  |    2 --
 3 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index ded3c55..2b86cdf 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -72,7 +72,6 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries,
         new_size = bar_size * 2;
     }
 
-    pdev->msix_bar_size = new_size;
     config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX,
                                        0, MSIX_CAP_LENGTH);
     if (config_offset < 0)
@@ -382,13 +381,6 @@ int msix_enabled(PCIDevice *dev)
          MSIX_ENABLE_MASK);
 }
 
-/* Size of bar where MSI-X table resides, or 0 if MSI-X not supported. */
-uint32_t msix_bar_size(PCIDevice *dev)
-{
-    return (dev->cap_present & QEMU_PCI_CAP_MSIX) ?
-        dev->msix_bar_size : 0;
-}
-
 /* Send an MSI-X message */
 void msix_notify(PCIDevice *dev, unsigned vector)
 {
diff --git a/hw/msix.h b/hw/msix.h
index 50aee82..e5a488d 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -21,8 +21,6 @@ void msix_load(PCIDevice *dev, QEMUFile *f);
 int msix_enabled(PCIDevice *dev);
 int msix_present(PCIDevice *dev);
 
-uint32_t msix_bar_size(PCIDevice *dev);
-
 int msix_vector_use(PCIDevice *dev, unsigned vector);
 void msix_vector_unuse(PCIDevice *dev, unsigned vector);
 void msix_unuse_all_vectors(PCIDevice *dev);
diff --git a/hw/pci.h b/hw/pci.h
index c3cacce..3d534e7 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -226,8 +226,6 @@ struct PCIDevice {
     MemoryRegion msix_mmio;
     /* Reference-count for entries actually in use by driver. */
     unsigned *msix_entry_used;
-    /* Region including the MSI-X table */
-    uint32_t msix_bar_size;
     /* MSIX function mask set or MSIX disabled */
     bool msix_function_masked;
     /* Version id needed for VMState */
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH] msix: Drop unused msix_bar_size

[Michael S. Tsirkin]

On Mon, Jun 04, 2012 at 04:56:01PM +0200, Jan Kiszka wrote:
> No user in sight.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Applied.

> ---
>  hw/msix.c |    8 --------
>  hw/msix.h |    2 --
>  hw/pci.h  |    2 --
>  3 files changed, 0 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index ded3c55..2b86cdf 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -72,7 +72,6 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries,
>          new_size = bar_size * 2;
>      }
>  
> -    pdev->msix_bar_size = new_size;
>      config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX,
>                                         0, MSIX_CAP_LENGTH);
>      if (config_offset < 0)
> @@ -382,13 +381,6 @@ int msix_enabled(PCIDevice *dev)
>           MSIX_ENABLE_MASK);
>  }
>  
> -/* Size of bar where MSI-X table resides, or 0 if MSI-X not supported. */
> -uint32_t msix_bar_size(PCIDevice *dev)
> -{
> -    return (dev->cap_present & QEMU_PCI_CAP_MSIX) ?
> -        dev->msix_bar_size : 0;
> -}
> -
>  /* Send an MSI-X message */
>  void msix_notify(PCIDevice *dev, unsigned vector)
>  {
> diff --git a/hw/msix.h b/hw/msix.h
> index 50aee82..e5a488d 100644
> --- a/hw/msix.h
> +++ b/hw/msix.h
> @@ -21,8 +21,6 @@ void msix_load(PCIDevice *dev, QEMUFile *f);
>  int msix_enabled(PCIDevice *dev);
>  int msix_present(PCIDevice *dev);
>  
> -uint32_t msix_bar_size(PCIDevice *dev);
> -
>  int msix_vector_use(PCIDevice *dev, unsigned vector);
>  void msix_vector_unuse(PCIDevice *dev, unsigned vector);
>  void msix_unuse_all_vectors(PCIDevice *dev);
> diff --git a/hw/pci.h b/hw/pci.h
> index c3cacce..3d534e7 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -226,8 +226,6 @@ struct PCIDevice {
>      MemoryRegion msix_mmio;
>      /* Reference-count for entries actually in use by driver. */
>      unsigned *msix_entry_used;
> -    /* Region including the MSI-X table */
> -    uint32_t msix_bar_size;
>      /* MSIX function mask set or MSIX disabled */
>      bool msix_function_masked;
>      /* Version id needed for VMState */
> -- 
> 1.7.3.4

Re: [Qemu-devel] [PATCH] msix: Drop unused msix_bar_size

[Michael S. Tsirkin]

On Mon, Jun 04, 2012 at 04:56:01PM +0200, Jan Kiszka wrote:
> No user in sight.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Just to note, currently (since the memory API changes) all users assume
a single 4K page for MSI-X, we will need some API to remove that
assumption.
Still the patch is fine.

> ---
>  hw/msix.c |    8 --------
>  hw/msix.h |    2 --
>  hw/pci.h  |    2 --
>  3 files changed, 0 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index ded3c55..2b86cdf 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -72,7 +72,6 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries,
>          new_size = bar_size * 2;
>      }
>  
> -    pdev->msix_bar_size = new_size;
>      config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX,
>                                         0, MSIX_CAP_LENGTH);
>      if (config_offset < 0)
> @@ -382,13 +381,6 @@ int msix_enabled(PCIDevice *dev)
>           MSIX_ENABLE_MASK);
>  }
>  
> -/* Size of bar where MSI-X table resides, or 0 if MSI-X not supported. */
> -uint32_t msix_bar_size(PCIDevice *dev)
> -{
> -    return (dev->cap_present & QEMU_PCI_CAP_MSIX) ?
> -        dev->msix_bar_size : 0;
> -}
> -
>  /* Send an MSI-X message */
>  void msix_notify(PCIDevice *dev, unsigned vector)
>  {
> diff --git a/hw/msix.h b/hw/msix.h
> index 50aee82..e5a488d 100644
> --- a/hw/msix.h
> +++ b/hw/msix.h
> @@ -21,8 +21,6 @@ void msix_load(PCIDevice *dev, QEMUFile *f);
>  int msix_enabled(PCIDevice *dev);
>  int msix_present(PCIDevice *dev);
>  
> -uint32_t msix_bar_size(PCIDevice *dev);
> -
>  int msix_vector_use(PCIDevice *dev, unsigned vector);
>  void msix_vector_unuse(PCIDevice *dev, unsigned vector);
>  void msix_unuse_all_vectors(PCIDevice *dev);
> diff --git a/hw/pci.h b/hw/pci.h
> index c3cacce..3d534e7 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -226,8 +226,6 @@ struct PCIDevice {
>      MemoryRegion msix_mmio;
>      /* Reference-count for entries actually in use by driver. */
>      unsigned *msix_entry_used;
> -    /* Region including the MSI-X table */
> -    uint32_t msix_bar_size;
>      /* MSIX function mask set or MSIX disabled */
>      bool msix_function_masked;
>      /* Version id needed for VMState */
> -- 
> 1.7.3.4

Re: [Qemu-devel] [PATCH] msix: Drop unused msix_bar_size

[Jan Kiszka]

On 2012-06-05 12:20, Michael S. Tsirkin wrote:
> On Mon, Jun 04, 2012 at 04:56:01PM +0200, Jan Kiszka wrote:
>> No user in sight.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Just to note, currently (since the memory API changes) all users assume
> a single 4K page for MSI-X, we will need some API to remove that
> assumption.
> Still the patch is fine.

Yep, I have a patch for this here (I think I posted an earlier version
before). It's required to do device assignment using the MSI layer.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] msix: Drop unused msix_bar_size

[Alex Williamson]

On Tue, 2012-06-05 at 12:26 +0200, Jan Kiszka wrote:
> On 2012-06-05 12:20, Michael S. Tsirkin wrote:
> > On Mon, Jun 04, 2012 at 04:56:01PM +0200, Jan Kiszka wrote:
> >> No user in sight.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Just to note, currently (since the memory API changes) all users assume
> > a single 4K page for MSI-X, we will need some API to remove that
> > assumption.
> > Still the patch is fine.
> 
> Yep, I have a patch for this here (I think I posted an earlier version
> before). It's required to do device assignment using the MSI layer.

Would you mind sending a pointer or reposting, I can't find it.  Trying
to switch vfio to not require existing capabilities checks I was
reminded of the horror of msix_init().  We need a pretty good overhaul
here to be able to allow the flexibility of defining where both the
vector table a pba live to make it more compatible with real devices.  I
honestly don't see the benefit of allowing msix_init() the ability to
resize a BAR to give itself room.  There are only two other users
currently, ivshmem and virtio, both of which allocate a BAR exclusively
for MSI-X.  Thanks,

Alex