Patchwork [v2,9/9] msi: Move PCI_MSI_PENDING_32/64 into pci_regs.h

login
register
mail settings
Submitter Jan Kiszka
Date June 8, 2011, 4:21 p.m.
Message ID <b653b747d8b3107a16e491464ea6669129845367.1307550106.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/99518/
State New
Headers show

Comments

Jan Kiszka - June 8, 2011, 4:21 p.m.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/msi.c      |    4 ----
 hw/pci_regs.h |    2 ++
 2 files changed, 2 insertions(+), 4 deletions(-)
Michael S. Tsirkin - June 8, 2011, 7:48 p.m.
On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

pci_regs.h from linux does not have these
this is why we keep them in msi.c

[mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
#define PCI_MSI_FLAGS           2       /* Various flags */
#define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
#define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
*/
#define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
*/
#define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
#define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
#define PCI_MSI_RFU             3       /* Rest of capability flags */
#define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
#define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
PCI_MSI_FLAGS_64BIT set) */
#define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
devices */
#define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
devices */
#define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
devices */
#define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
devices */


If you want to move them, please send them upstream we'll merge when
they are there.

> ---
>  hw/msi.c      |    4 ----
>  hw/pci_regs.h |    2 ++
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/msi.c b/hw/msi.c
> index e23f5df..d548939 100644
> --- a/hw/msi.c
> +++ b/hw/msi.c
> @@ -21,10 +21,6 @@
>  #include "msi.h"
>  #include "range.h"
>  
> -/* Eventually those constants should go to Linux pci_regs.h */
> -#define PCI_MSI_PENDING_32      0x10
> -#define PCI_MSI_PENDING_64      0x14
> -
>  /* PCI_MSI_ADDRESS_LO */
>  #define PCI_MSI_ADDRESS_LO_MASK         (~0x3)
>  
> diff --git a/hw/pci_regs.h b/hw/pci_regs.h
> index c17c22f..002ed2e 100644
> --- a/hw/pci_regs.h
> +++ b/hw/pci_regs.h
> @@ -297,8 +297,10 @@
>  #define PCI_MSI_ADDRESS_HI	8	/* Upper 32 bits (if PCI_MSI_FLAGS_64BIT set) */
>  #define PCI_MSI_DATA_32		8	/* 16 bits of data for 32-bit devices */
>  #define PCI_MSI_MASK_32		12	/* Mask bits register for 32-bit devices */
> +#define PCI_MSI_PENDING_32      16      /* Pending bits register for 32-bit devices */
>  #define PCI_MSI_DATA_64		12	/* 16 bits of data for 64-bit devices */
>  #define PCI_MSI_MASK_64		16	/* Mask bits register for 64-bit devices */
> +#define PCI_MSI_PENDING_64      20      /* Pending bits register for 64-bit devices */
>  
>  /* MSI-X registers */
>  #define PCI_MSIX_CTRL           2       /* Message control */
> -- 
> 1.7.1
Jan Kiszka - June 8, 2011, 8:44 p.m.
On 2011-06-08 21:48, Michael S. Tsirkin wrote:
> On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> pci_regs.h from linux does not have these
> this is why we keep them in msi.c
> 
> [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
> #define PCI_MSI_FLAGS           2       /* Various flags */
> #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
> #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
> */
> #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
> */
> #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
> #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
> #define PCI_MSI_RFU             3       /* Rest of capability flags */
> #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
> #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
> PCI_MSI_FLAGS_64BIT set) */
> #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
> devices */
> #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
> devices */
> #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
> devices */
> #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
> devices */
> 
> 
> If you want to move them, please send them upstream we'll merge when
> they are there.

In fact, both defines are already in libpci. Since 3.0.0. Released 5
years ago. OK, I'll send a header resync patch against a more recent
release.

Then we should just lack something like PCI_MSIX_CTRL. I will have a look.

Jan
Michael S. Tsirkin - June 8, 2011, 8:56 p.m.
On Wed, Jun 08, 2011 at 10:44:58PM +0200, Jan Kiszka wrote:
> On 2011-06-08 21:48, Michael S. Tsirkin wrote:
> > On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > pci_regs.h from linux does not have these
> > this is why we keep them in msi.c
> > 
> > [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
> > #define PCI_MSI_FLAGS           2       /* Various flags */
> > #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
> > #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
> > */
> > #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
> > */
> > #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
> > #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
> > #define PCI_MSI_RFU             3       /* Rest of capability flags */
> > #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
> > #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
> > PCI_MSI_FLAGS_64BIT set) */
> > #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
> > devices */
> > #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
> > devices */
> > #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
> > devices */
> > #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
> > devices */
> > 
> > 
> > If you want to move them, please send them upstream we'll merge when
> > they are there.
> 
> In fact, both defines are already in libpci. Since 3.0.0. Released 5
> years ago. OK, I'll send a header resync patch against a more recent
> release.
> 
> Then we should just lack something like PCI_MSIX_CTRL. I will have a look.

PCI_MSIX_FLAGS is the same.

> 
> Jan
>
Jan Kiszka - June 8, 2011, 8:57 p.m.
On 2011-06-08 22:56, Michael S. Tsirkin wrote:
> On Wed, Jun 08, 2011 at 10:44:58PM +0200, Jan Kiszka wrote:
>> On 2011-06-08 21:48, Michael S. Tsirkin wrote:
>>> On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> pci_regs.h from linux does not have these
>>> this is why we keep them in msi.c
>>>
>>> [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
>>> #define PCI_MSI_FLAGS           2       /* Various flags */
>>> #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
>>> #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
>>> */
>>> #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
>>> */
>>> #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
>>> #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
>>> #define PCI_MSI_RFU             3       /* Rest of capability flags */
>>> #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
>>> #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
>>> PCI_MSI_FLAGS_64BIT set) */
>>> #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
>>> devices */
>>> #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
>>> devices */
>>> #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
>>> devices */
>>> #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
>>> devices */
>>>
>>>
>>> If you want to move them, please send them upstream we'll merge when
>>> they are there.
>>
>> In fact, both defines are already in libpci. Since 3.0.0. Released 5
>> years ago. OK, I'll send a header resync patch against a more recent
>> release.
>>
>> Then we should just lack something like PCI_MSIX_CTRL. I will have a look.
> 
> PCI_MSIX_FLAGS is the same.

That does not exist in libpci's header.

Jan
Michael S. Tsirkin - June 8, 2011, 9:01 p.m.
On Wed, Jun 08, 2011 at 10:57:13PM +0200, Jan Kiszka wrote:
> On 2011-06-08 22:56, Michael S. Tsirkin wrote:
> > On Wed, Jun 08, 2011 at 10:44:58PM +0200, Jan Kiszka wrote:
> >> On 2011-06-08 21:48, Michael S. Tsirkin wrote:
> >>> On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>
> >>> pci_regs.h from linux does not have these
> >>> this is why we keep them in msi.c
> >>>
> >>> [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
> >>> #define PCI_MSI_FLAGS           2       /* Various flags */
> >>> #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
> >>> #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
> >>> */
> >>> #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
> >>> */
> >>> #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
> >>> #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
> >>> #define PCI_MSI_RFU             3       /* Rest of capability flags */
> >>> #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
> >>> #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
> >>> PCI_MSI_FLAGS_64BIT set) */
> >>> #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
> >>> devices */
> >>> #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
> >>> devices */
> >>> #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
> >>> devices */
> >>> #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
> >>> devices */
> >>>
> >>>
> >>> If you want to move them, please send them upstream we'll merge when
> >>> they are there.
> >>
> >> In fact, both defines are already in libpci. Since 3.0.0. Released 5
> >> years ago. OK, I'll send a header resync patch against a more recent
> >> release.
> >>
> >> Then we should just lack something like PCI_MSIX_CTRL. I will have a look.
> > 
> > PCI_MSIX_FLAGS is the same.
> 
> That does not exist in libpci's header.
> 
> Jan
> 

The upstream I meant is pci_Regs.h in Linux.
Jan Kiszka - June 8, 2011, 9:03 p.m.
On 2011-06-08 23:01, Michael S. Tsirkin wrote:
> On Wed, Jun 08, 2011 at 10:57:13PM +0200, Jan Kiszka wrote:
>> On 2011-06-08 22:56, Michael S. Tsirkin wrote:
>>> On Wed, Jun 08, 2011 at 10:44:58PM +0200, Jan Kiszka wrote:
>>>> On 2011-06-08 21:48, Michael S. Tsirkin wrote:
>>>>> On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>
>>>>> pci_regs.h from linux does not have these
>>>>> this is why we keep them in msi.c
>>>>>
>>>>> [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
>>>>> #define PCI_MSI_FLAGS           2       /* Various flags */
>>>>> #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
>>>>> #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
>>>>> */
>>>>> #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
>>>>> */
>>>>> #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
>>>>> #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
>>>>> #define PCI_MSI_RFU             3       /* Rest of capability flags */
>>>>> #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
>>>>> #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
>>>>> PCI_MSI_FLAGS_64BIT set) */
>>>>> #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
>>>>> devices */
>>>>> #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
>>>>> devices */
>>>>> #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
>>>>> devices */
>>>>> #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
>>>>> devices */
>>>>>
>>>>>
>>>>> If you want to move them, please send them upstream we'll merge when
>>>>> they are there.
>>>>
>>>> In fact, both defines are already in libpci. Since 3.0.0. Released 5
>>>> years ago. OK, I'll send a header resync patch against a more recent
>>>> release.
>>>>
>>>> Then we should just lack something like PCI_MSIX_CTRL. I will have a look.
>>>
>>> PCI_MSIX_FLAGS is the same.
>>
>> That does not exist in libpci's header.
>>
>> Jan
>>
> 
> The upstream I meant is pci_Regs.h in Linux.
> 

Why not sync against
git://git.kernel.org/pub/scm/utils/pciutils/pciutils.git? Sounds more
appropriate.

Jan
Michael S. Tsirkin - June 8, 2011, 9:14 p.m.
On Wed, Jun 08, 2011 at 11:03:43PM +0200, Jan Kiszka wrote:
> On 2011-06-08 23:01, Michael S. Tsirkin wrote:
> > On Wed, Jun 08, 2011 at 10:57:13PM +0200, Jan Kiszka wrote:
> >> On 2011-06-08 22:56, Michael S. Tsirkin wrote:
> >>> On Wed, Jun 08, 2011 at 10:44:58PM +0200, Jan Kiszka wrote:
> >>>> On 2011-06-08 21:48, Michael S. Tsirkin wrote:
> >>>>> On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
> >>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>
> >>>>> pci_regs.h from linux does not have these
> >>>>> this is why we keep them in msi.c
> >>>>>
> >>>>> [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
> >>>>> #define PCI_MSI_FLAGS           2       /* Various flags */
> >>>>> #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
> >>>>> #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
> >>>>> */
> >>>>> #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
> >>>>> */
> >>>>> #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
> >>>>> #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
> >>>>> #define PCI_MSI_RFU             3       /* Rest of capability flags */
> >>>>> #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
> >>>>> #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
> >>>>> PCI_MSI_FLAGS_64BIT set) */
> >>>>> #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
> >>>>> devices */
> >>>>> #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
> >>>>> devices */
> >>>>> #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
> >>>>> devices */
> >>>>> #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
> >>>>> devices */
> >>>>>
> >>>>>
> >>>>> If you want to move them, please send them upstream we'll merge when
> >>>>> they are there.
> >>>>
> >>>> In fact, both defines are already in libpci. Since 3.0.0. Released 5
> >>>> years ago. OK, I'll send a header resync patch against a more recent
> >>>> release.
> >>>>
> >>>> Then we should just lack something like PCI_MSIX_CTRL. I will have a look.
> >>>
> >>> PCI_MSIX_FLAGS is the same.
> >>
> >> That does not exist in libpci's header.
> >>
> >> Jan
> >>
> > 
> > The upstream I meant is pci_Regs.h in Linux.
> > 
> 
> Why not sync against
> git://git.kernel.org/pub/scm/utils/pciutils/pciutils.git? Sounds more
> appropriate.
> 
> Jan
> 

We are 100% aligned with Linux changing that seems pointless.
I think Linux gets new hardware features faster and has a better
chance to be correct.
We already pull headers from Linux so that's one dependency less.
Jan Kiszka - June 8, 2011, 9:18 p.m.
On 2011-06-08 23:14, Michael S. Tsirkin wrote:
> On Wed, Jun 08, 2011 at 11:03:43PM +0200, Jan Kiszka wrote:
>> On 2011-06-08 23:01, Michael S. Tsirkin wrote:
>>> On Wed, Jun 08, 2011 at 10:57:13PM +0200, Jan Kiszka wrote:
>>>> On 2011-06-08 22:56, Michael S. Tsirkin wrote:
>>>>> On Wed, Jun 08, 2011 at 10:44:58PM +0200, Jan Kiszka wrote:
>>>>>> On 2011-06-08 21:48, Michael S. Tsirkin wrote:
>>>>>>> On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>
>>>>>>> pci_regs.h from linux does not have these
>>>>>>> this is why we keep them in msi.c
>>>>>>>
>>>>>>> [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
>>>>>>> #define PCI_MSI_FLAGS           2       /* Various flags */
>>>>>>> #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
>>>>>>> #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
>>>>>>> */
>>>>>>> #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
>>>>>>> */
>>>>>>> #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
>>>>>>> #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
>>>>>>> #define PCI_MSI_RFU             3       /* Rest of capability flags */
>>>>>>> #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
>>>>>>> #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
>>>>>>> PCI_MSI_FLAGS_64BIT set) */
>>>>>>> #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
>>>>>>> devices */
>>>>>>> #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
>>>>>>> devices */
>>>>>>> #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
>>>>>>> devices */
>>>>>>> #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
>>>>>>> devices */
>>>>>>>
>>>>>>>
>>>>>>> If you want to move them, please send them upstream we'll merge when
>>>>>>> they are there.
>>>>>>
>>>>>> In fact, both defines are already in libpci. Since 3.0.0. Released 5
>>>>>> years ago. OK, I'll send a header resync patch against a more recent
>>>>>> release.
>>>>>>
>>>>>> Then we should just lack something like PCI_MSIX_CTRL. I will have a look.
>>>>>
>>>>> PCI_MSIX_FLAGS is the same.
>>>>
>>>> That does not exist in libpci's header.
>>>>
>>>> Jan
>>>>
>>>
>>> The upstream I meant is pci_Regs.h in Linux.
>>>
>>
>> Why not sync against
>> git://git.kernel.org/pub/scm/utils/pciutils/pciutils.git? Sounds more
>> appropriate.
>>
>> Jan
>>
> 
> We are 100% aligned with Linux changing that seems pointless.
> I think Linux gets new hardware features faster and has a better
> chance to be correct.
> We already pull headers from Linux so that's one dependency less.
> 

The kernel appears to gain defines based on what it uses. The libpci
headers seem to serve more use cases, at least it had the missing fields
for several years and is a few hundred lines longer.

Jan
Jan Kiszka - June 8, 2011, 9:24 p.m.
On 2011-06-08 23:18, Jan Kiszka wrote:
> On 2011-06-08 23:14, Michael S. Tsirkin wrote:
>> On Wed, Jun 08, 2011 at 11:03:43PM +0200, Jan Kiszka wrote:
>>> On 2011-06-08 23:01, Michael S. Tsirkin wrote:
>>>> On Wed, Jun 08, 2011 at 10:57:13PM +0200, Jan Kiszka wrote:
>>>>> On 2011-06-08 22:56, Michael S. Tsirkin wrote:
>>>>>> On Wed, Jun 08, 2011 at 10:44:58PM +0200, Jan Kiszka wrote:
>>>>>>> On 2011-06-08 21:48, Michael S. Tsirkin wrote:
>>>>>>>> On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>
>>>>>>>> pci_regs.h from linux does not have these
>>>>>>>> this is why we keep them in msi.c
>>>>>>>>
>>>>>>>> [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
>>>>>>>> #define PCI_MSI_FLAGS           2       /* Various flags */
>>>>>>>> #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
>>>>>>>> #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
>>>>>>>> */
>>>>>>>> #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
>>>>>>>> */
>>>>>>>> #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
>>>>>>>> #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
>>>>>>>> #define PCI_MSI_RFU             3       /* Rest of capability flags */
>>>>>>>> #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
>>>>>>>> #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
>>>>>>>> PCI_MSI_FLAGS_64BIT set) */
>>>>>>>> #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
>>>>>>>> devices */
>>>>>>>> #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
>>>>>>>> devices */
>>>>>>>> #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
>>>>>>>> devices */
>>>>>>>> #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
>>>>>>>> devices */
>>>>>>>>
>>>>>>>>
>>>>>>>> If you want to move them, please send them upstream we'll merge when
>>>>>>>> they are there.
>>>>>>>
>>>>>>> In fact, both defines are already in libpci. Since 3.0.0. Released 5
>>>>>>> years ago. OK, I'll send a header resync patch against a more recent
>>>>>>> release.
>>>>>>>
>>>>>>> Then we should just lack something like PCI_MSIX_CTRL. I will have a look.
>>>>>>
>>>>>> PCI_MSIX_FLAGS is the same.
>>>>>
>>>>> That does not exist in libpci's header.
>>>>>
>>>>> Jan
>>>>>
>>>>
>>>> The upstream I meant is pci_Regs.h in Linux.
>>>>
>>>
>>> Why not sync against
>>> git://git.kernel.org/pub/scm/utils/pciutils/pciutils.git? Sounds more
>>> appropriate.
>>>
>>> Jan
>>>
>>
>> We are 100% aligned with Linux changing that seems pointless.
>> I think Linux gets new hardware features faster and has a better
>> chance to be correct.
>> We already pull headers from Linux so that's one dependency less.
>>
> 
> The kernel appears to gain defines based on what it uses. The libpci
> headers seem to serve more use cases, at least it had the missing fields
> for several years and is a few hundred lines longer.

OK, looks like that "added value" comes from merging pci_regs.h with
pci_ids.h. Never mind, will work against the kernel.

Jan
Michael S. Tsirkin - June 8, 2011, 9:30 p.m.
On Wed, Jun 08, 2011 at 11:18:05PM +0200, Jan Kiszka wrote:
> On 2011-06-08 23:14, Michael S. Tsirkin wrote:
> > On Wed, Jun 08, 2011 at 11:03:43PM +0200, Jan Kiszka wrote:
> >> On 2011-06-08 23:01, Michael S. Tsirkin wrote:
> >>> On Wed, Jun 08, 2011 at 10:57:13PM +0200, Jan Kiszka wrote:
> >>>> On 2011-06-08 22:56, Michael S. Tsirkin wrote:
> >>>>> On Wed, Jun 08, 2011 at 10:44:58PM +0200, Jan Kiszka wrote:
> >>>>>> On 2011-06-08 21:48, Michael S. Tsirkin wrote:
> >>>>>>> On Wed, Jun 08, 2011 at 06:21:52PM +0200, Jan Kiszka wrote:
> >>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>
> >>>>>>> pci_regs.h from linux does not have these
> >>>>>>> this is why we keep them in msi.c
> >>>>>>>
> >>>>>>> [mst@tuck linux-2.6]$ grep PCI_MSI_ include/linux/pci_regs.h 
> >>>>>>> #define PCI_MSI_FLAGS           2       /* Various flags */
> >>>>>>> #define  PCI_MSI_FLAGS_64BIT    0x80    /* 64-bit addresses allowed */
> >>>>>>> #define  PCI_MSI_FLAGS_QSIZE    0x70    /* Message queue size configured
> >>>>>>> */
> >>>>>>> #define  PCI_MSI_FLAGS_QMASK    0x0e    /* Maximum queue size available
> >>>>>>> */
> >>>>>>> #define  PCI_MSI_FLAGS_ENABLE   0x01    /* MSI feature enabled */
> >>>>>>> #define  PCI_MSI_FLAGS_MASKBIT  0x100   /* 64-bit mask bits allowed */
> >>>>>>> #define PCI_MSI_RFU             3       /* Rest of capability flags */
> >>>>>>> #define PCI_MSI_ADDRESS_LO      4       /* Lower 32 bits */
> >>>>>>> #define PCI_MSI_ADDRESS_HI      8       /* Upper 32 bits (if
> >>>>>>> PCI_MSI_FLAGS_64BIT set) */
> >>>>>>> #define PCI_MSI_DATA_32         8       /* 16 bits of data for 32-bit
> >>>>>>> devices */
> >>>>>>> #define PCI_MSI_MASK_32         12      /* Mask bits register for 32-bit
> >>>>>>> devices */
> >>>>>>> #define PCI_MSI_DATA_64         12      /* 16 bits of data for 64-bit
> >>>>>>> devices */
> >>>>>>> #define PCI_MSI_MASK_64         16      /* Mask bits register for 64-bit
> >>>>>>> devices */
> >>>>>>>
> >>>>>>>
> >>>>>>> If you want to move them, please send them upstream we'll merge when
> >>>>>>> they are there.
> >>>>>>
> >>>>>> In fact, both defines are already in libpci. Since 3.0.0. Released 5
> >>>>>> years ago. OK, I'll send a header resync patch against a more recent
> >>>>>> release.
> >>>>>>
> >>>>>> Then we should just lack something like PCI_MSIX_CTRL. I will have a look.
> >>>>>
> >>>>> PCI_MSIX_FLAGS is the same.
> >>>>
> >>>> That does not exist in libpci's header.
> >>>>
> >>>> Jan
> >>>>
> >>>
> >>> The upstream I meant is pci_Regs.h in Linux.
> >>>
> >>
> >> Why not sync against
> >> git://git.kernel.org/pub/scm/utils/pciutils/pciutils.git? Sounds more
> >> appropriate.
> >>
> >> Jan
> >>
> > 
> > We are 100% aligned with Linux changing that seems pointless.
> > I think Linux gets new hardware features faster and has a better
> > chance to be correct.
> > We already pull headers from Linux so that's one dependency less.
> > 
> 
> The kernel appears to gain defines based on what it uses.

I don't expect trouble adding stuff we might need though.

> The libpci
> headers seem to serve more use cases, at least it had the missing fields
> for several years and is a few hundred lines longer.
> 
> Jan
> 

Some defines got into Linux already, specifically
MSIX_TABLE_OFFSET, MSIX_PBA_OFFSET
(with different names)

Patch

diff --git a/hw/msi.c b/hw/msi.c
index e23f5df..d548939 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -21,10 +21,6 @@ 
 #include "msi.h"
 #include "range.h"
 
-/* Eventually those constants should go to Linux pci_regs.h */
-#define PCI_MSI_PENDING_32      0x10
-#define PCI_MSI_PENDING_64      0x14
-
 /* PCI_MSI_ADDRESS_LO */
 #define PCI_MSI_ADDRESS_LO_MASK         (~0x3)
 
diff --git a/hw/pci_regs.h b/hw/pci_regs.h
index c17c22f..002ed2e 100644
--- a/hw/pci_regs.h
+++ b/hw/pci_regs.h
@@ -297,8 +297,10 @@ 
 #define PCI_MSI_ADDRESS_HI	8	/* Upper 32 bits (if PCI_MSI_FLAGS_64BIT set) */
 #define PCI_MSI_DATA_32		8	/* 16 bits of data for 32-bit devices */
 #define PCI_MSI_MASK_32		12	/* Mask bits register for 32-bit devices */
+#define PCI_MSI_PENDING_32      16      /* Pending bits register for 32-bit devices */
 #define PCI_MSI_DATA_64		12	/* 16 bits of data for 64-bit devices */
 #define PCI_MSI_MASK_64		16	/* Mask bits register for 64-bit devices */
+#define PCI_MSI_PENDING_64      20      /* Pending bits register for 64-bit devices */
 
 /* MSI-X registers */
 #define PCI_MSIX_CTRL           2       /* Message control */