On 12/13/2017 9:58 AM, Alex Williamson wrote:
On Wed, 13 Dec 2017 15:13:55 +0800
Peter Xu <pet...@redhat.com> wrote:
On Tue, Dec 12, 2017 at 03:43:08PM -0700, Alex Williamson wrote:
[...]
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 9a7ffd13c7f0..87888b102057 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1345,7 +1345,9 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16
sid, u16 qdep,
struct qi_desc desc;
if (mask) {
- BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1));
+ BUG_ON((mask > MAX_AGAW_PFN_WIDTH) ||
+ ((mask == MAX_AGAW_PFN_WIDTH) && addr) ||
+ (addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1)));
Could it work if we just use 1ULL instead of 1 here? Thanks,
In either case we're talking about shifting off the end of the
variable, which I understand to be undefined. Right? Thanks,
How so? Bits fall off the left (MSB) end, zeroes fill in the right (LSB)
end. I believe that behavior is pretty set.
The only problem here is that the promotion of integral types is (at the
very least) unclear in this statement. As for the proposal, do we know
that 1ULL is always going to be the same size as addr?
I would suggest, as pedantic as it sounds, creating a local u64
variable, initialized to 1, and using that in the BUG_ON:
u64 ubit = 1;
...
if (mask) {
BUG_ON(addr & ((ubit << (VTD_PAGE_SHIFT + mask)) - 1));
I believe this forces everything to be appropriately wide, and the same
size?
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu