diff mbox series

[kvm-unit-tests,2/7] pci: use uint32_t for unsigned long values

Message ID 20200226094433.210968-4-morbo@google.com (mailing list archive)
State New, archived
Headers show
Series Fixes for clang builds | expand

Commit Message

Bill Wendling Feb. 26, 2020, 9:44 a.m. UTC
The "pci_bar_*" functions use 64-bit masks, but the results are assigned
to 32-bit variables. Use 32-bit masks, since we're interested only in
the least significant 4-bits.

Signed-off-by: Bill Wendling <morbo@google.com>
---
 lib/linux/pci_regs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini Feb. 28, 2020, 11:04 a.m. UTC | #1
On 26/02/20 10:44, Bill Wendling wrote:
> The "pci_bar_*" functions use 64-bit masks, but the results are assigned
> to 32-bit variables. Use 32-bit masks, since we're interested only in
> the least significant 4-bits.
> 
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
>  lib/linux/pci_regs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/linux/pci_regs.h b/lib/linux/pci_regs.h
> index 1becea8..3bc2b92 100644
> --- a/lib/linux/pci_regs.h
> +++ b/lib/linux/pci_regs.h
> @@ -96,8 +96,8 @@
>  #define  PCI_BASE_ADDRESS_MEM_TYPE_1M	0x02	/* Below 1M [obsolete] */
>  #define  PCI_BASE_ADDRESS_MEM_TYPE_64	0x04	/* 64 bit address */
>  #define  PCI_BASE_ADDRESS_MEM_PREFETCH	0x08	/* prefetchable? */
> -#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fUL)
> -#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03UL)
> +#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fU)
> +#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03U)
>  /* bit 1 is reserved if address_space = 1 */
>  
>  /* Header type 0 (normal devices) */
> 

Removing the "U" is even better because it will then sign-extend
automatically.

Paolo
Andrew Jones Feb. 28, 2020, 12:46 p.m. UTC | #2
On Fri, Feb 28, 2020 at 12:04:38PM +0100, Paolo Bonzini wrote:
> On 26/02/20 10:44, Bill Wendling wrote:
> > The "pci_bar_*" functions use 64-bit masks, but the results are assigned
> > to 32-bit variables. Use 32-bit masks, since we're interested only in
> > the least significant 4-bits.
> > 
> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> >  lib/linux/pci_regs.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/linux/pci_regs.h b/lib/linux/pci_regs.h
> > index 1becea8..3bc2b92 100644
> > --- a/lib/linux/pci_regs.h
> > +++ b/lib/linux/pci_regs.h
> > @@ -96,8 +96,8 @@
> >  #define  PCI_BASE_ADDRESS_MEM_TYPE_1M	0x02	/* Below 1M [obsolete] */
> >  #define  PCI_BASE_ADDRESS_MEM_TYPE_64	0x04	/* 64 bit address */
> >  #define  PCI_BASE_ADDRESS_MEM_PREFETCH	0x08	/* prefetchable? */
> > -#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fUL)
> > -#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03UL)
> > +#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fU)
> > +#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03U)
> >  /* bit 1 is reserved if address_space = 1 */
> >  
> >  /* Header type 0 (normal devices) */
> > 
> 
> Removing the "U" is even better because it will then sign-extend
> automatically.
>

We don't want this patch at all though. We shouldn't change pci_regs.h
since it comes from linux and someday we may update again and lose
any changes we make. We should change how these masks are used instead.

Thanks,
drew
Paolo Bonzini Feb. 28, 2020, 1:02 p.m. UTC | #3
On 28/02/20 13:46, Andrew Jones wrote:
> On Fri, Feb 28, 2020 at 12:04:38PM +0100, Paolo Bonzini wrote:
>> On 26/02/20 10:44, Bill Wendling wrote:
>>> The "pci_bar_*" functions use 64-bit masks, but the results are assigned
>>> to 32-bit variables. Use 32-bit masks, since we're interested only in
>>> the least significant 4-bits.
>>>
>>> Signed-off-by: Bill Wendling <morbo@google.com>
>>> ---
>>>  lib/linux/pci_regs.h | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/linux/pci_regs.h b/lib/linux/pci_regs.h
>>> index 1becea8..3bc2b92 100644
>>> --- a/lib/linux/pci_regs.h
>>> +++ b/lib/linux/pci_regs.h
>>> @@ -96,8 +96,8 @@
>>>  #define  PCI_BASE_ADDRESS_MEM_TYPE_1M	0x02	/* Below 1M [obsolete] */
>>>  #define  PCI_BASE_ADDRESS_MEM_TYPE_64	0x04	/* 64 bit address */
>>>  #define  PCI_BASE_ADDRESS_MEM_PREFETCH	0x08	/* prefetchable? */
>>> -#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fUL)
>>> -#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03UL)
>>> +#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fU)
>>> +#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03U)
>>>  /* bit 1 is reserved if address_space = 1 */
>>>  
>>>  /* Header type 0 (normal devices) */
>>>
>>
>> Removing the "U" is even better because it will then sign-extend
>> automatically.
>>
> 
> We don't want this patch at all though. We shouldn't change pci_regs.h
> since it comes from linux and someday we may update again and lose
> any changes we make. We should change how these masks are used instead.

I will try to get the change into Linux.

Paolo
diff mbox series

Patch

diff --git a/lib/linux/pci_regs.h b/lib/linux/pci_regs.h
index 1becea8..3bc2b92 100644
--- a/lib/linux/pci_regs.h
+++ b/lib/linux/pci_regs.h
@@ -96,8 +96,8 @@ 
 #define  PCI_BASE_ADDRESS_MEM_TYPE_1M	0x02	/* Below 1M [obsolete] */
 #define  PCI_BASE_ADDRESS_MEM_TYPE_64	0x04	/* 64 bit address */
 #define  PCI_BASE_ADDRESS_MEM_PREFETCH	0x08	/* prefetchable? */
-#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fUL)
-#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03UL)
+#define  PCI_BASE_ADDRESS_MEM_MASK	(~0x0fU)
+#define  PCI_BASE_ADDRESS_IO_MASK	(~0x03U)
 /* bit 1 is reserved if address_space = 1 */
 
 /* Header type 0 (normal devices) */