* [bug report] iommu/vt-d: Fix kdump kernels boot failure with scalable mode
@ 2022-09-15 10:05 Dan Carpenter
2022-09-15 10:21 ` Robin Murphy
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-09-15 10:05 UTC (permalink / raw)
To: baolu.lu; +Cc: Robin Murphy, iommu
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 but it will instead corrupt a bit way out
in the middle of your memory. 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);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] iommu/vt-d: Fix kdump kernels boot failure with scalable mode
2022-09-15 10:05 [bug report] iommu/vt-d: Fix kdump kernels boot failure with scalable mode Dan Carpenter
@ 2022-09-15 10:21 ` Robin Murphy
2022-09-15 10:44 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Robin Murphy @ 2022-09-15 10:21 UTC (permalink / raw)
To: Dan Carpenter, baolu.lu; +Cc: iommu
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] iommu/vt-d: Fix kdump kernels boot failure with scalable mode
2022-09-15 10:21 ` Robin Murphy
@ 2022-09-15 10:44 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2022-09-15 10:44 UTC (permalink / raw)
To: Robin Murphy; +Cc: baolu.lu, iommu
On Thu, Sep 15, 2022 at 11:21:36AM +0100, Robin Murphy wrote:
> 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.
>
Ah... Okay. That works then.
> > 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.
Btw, just BIT(16) works until you get above 31.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-15 10:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15 10:05 [bug report] iommu/vt-d: Fix kdump kernels boot failure with scalable mode Dan Carpenter
2022-09-15 10:21 ` Robin Murphy
2022-09-15 10:44 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).