Re: [bug report] iommu/vt-d: Fix kdump kernels boot failure with scalable mode

[Robin Murphy <robin.murphy@arm.com>]

On 2022-09-15 11:05, Dan Carpenter wrote:
> Hello Lu Baolu,
> 
> The patch 0c5f6c0d8201: "iommu/vt-d: Fix kdump kernels boot failure
> with scalable mode" from Aug 23, 2022, leads to the following Smatch
> static checker warning:
> 
> drivers/iommu/intel/iommu.c:224 set_context_copied() warn: set_bit() takes a bit number
> drivers/iommu/intel/iommu.c:230 clear_context_copied() warn: clear_bit() takes a bit number
> 
> drivers/iommu/intel/iommu.c
>      221 static inline void
>      222 set_context_copied(struct intel_iommu *iommu, u8 bus, u8 devfn)
>      223 {
> --> 224         set_bit(((long)bus << 8) | devfn, iommu->copied_tables);
>      225 }
> 
> This is trying to set a mask

No, it's simply composing a full 16-bit PCI requester ID from its two 
8-bit components.

> but it will instead corrupt a bit way out
> in the middle of your memory.

	iommu->copied_tables = bitmap_zalloc(BIT_ULL(16), GFP_KERNEL);

Again slightly non-obvious, but AFAICS the bitmap is sized appropriately.

>  The set_bit function will only set one
> bit at a time.  If we want to set bit zero:
> 
> 	set_bit(0, iommu->copied_tables);
> 
> Or if we have a whole page full of bits then we could set the last one:
> 
> 	set_bit(32767, page);

I believe that's exactly what we're doing here, setting one bit for each 
PCI ID processed. It's just a very big bitmap thanks to the size of the 
PCI ID space.

Cheers,
Robin.