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

Andrew Jones
2020-02-26T09:24:54Z

On Wed, Feb 26, 2020 at 01:02:39AM -0800, Bill Wendling wrote:
> I suspect then that how it's used in the kvm-unit-tests isn't correct.
> Or at least it's not how it's used in the Linux source code. Would it
> be better to cast all uses of these masks to uint32_t?

Yup. If clang is upset about kvm-unit-test's use of these masks, then
we need to change how they're used, as we can't change how they're
defined.

There's only one occurrence of each though, and in the same function,
pci_bar_mask(). I'd probably just change the return type of that
function to uint64_t, rather than add casts.

Thanks,
drew

>
> -bw
>
>
> On Tue, Feb 25, 2020 at 11:59 PM Andrew Jones <drjones@redhat.com> wrote:
> >
> > On Tue, Feb 25, 2020 at 11:44:22PM -0800, morbo@google.com wrote:
> > > From: Bill Wendling <morbo@google.com>
> > >
> > > 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) */
> > > --
> > > 2.25.0.265.gbab2e86ba0-goog
> > >
> >
> > This file comes directly from the Linux source. If it's not changed
> > there, then it shouldn't change here.
> >
> > Thanks,
> > drew
> >
>