All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE()
@ 2020-01-16  1:52 ` Lu Baolu
  0 siblings, 0 replies; 8+ messages in thread
From: Lu Baolu @ 2020-01-16  1:52 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: ashok.raj, jacob.jun.pan, kevin.tian, iommu, linux-kernel, Lu Baolu

Address field in device TLB invalidation descriptor is qualified
by the S field. If S field is zero, a single page at page address
specified by address [63:12] is requested to be invalidated. If S
field is set, the least significant bit in the address field with
value 0b (say bit N) indicates the invalidation address range. The
spec doesn't require the address [N - 1, 0] to be cleared, hence
remove the unnecessary WARN_ON_ONCE().

Otherwise, the caller might set "mask = MAX_AGAW_PFN_WIDTH" in order
to invalidating all the cached mappings on an endpoint, and below
overflow error will be triggered.

[...]
UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1354:3
shift exponent 64 is too large for 64-bit type 'long long unsigned int'
[...]

Reported-and-tested-by: Frank <fgndev@posteo.de>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/dmar.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 613b7153905d..071bb42bbbc5 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1354,7 +1354,6 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
 	struct qi_desc desc;
 
 	if (mask) {
-		WARN_ON_ONCE(addr & ((1ULL << (VTD_PAGE_SHIFT + mask)) - 1));
 		addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
 		desc.qw1 = QI_DEV_IOTLB_ADDR(addr) | QI_DEV_IOTLB_SIZE;
 	} else
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE()
@ 2020-01-16  1:52 ` Lu Baolu
  0 siblings, 0 replies; 8+ messages in thread
From: Lu Baolu @ 2020-01-16  1:52 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: kevin.tian, ashok.raj, linux-kernel, iommu, jacob.jun.pan

Address field in device TLB invalidation descriptor is qualified
by the S field. If S field is zero, a single page at page address
specified by address [63:12] is requested to be invalidated. If S
field is set, the least significant bit in the address field with
value 0b (say bit N) indicates the invalidation address range. The
spec doesn't require the address [N - 1, 0] to be cleared, hence
remove the unnecessary WARN_ON_ONCE().

Otherwise, the caller might set "mask = MAX_AGAW_PFN_WIDTH" in order
to invalidating all the cached mappings on an endpoint, and below
overflow error will be triggered.

[...]
UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1354:3
shift exponent 64 is too large for 64-bit type 'long long unsigned int'
[...]

Reported-and-tested-by: Frank <fgndev@posteo.de>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/dmar.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 613b7153905d..071bb42bbbc5 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1354,7 +1354,6 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
 	struct qi_desc desc;
 
 	if (mask) {
-		WARN_ON_ONCE(addr & ((1ULL << (VTD_PAGE_SHIFT + mask)) - 1));
 		addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
 		desc.qw1 = QI_DEV_IOTLB_ADDR(addr) | QI_DEV_IOTLB_SIZE;
 	} else
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE()
  2020-01-16  1:52 ` Lu Baolu
@ 2020-01-17  9:59   ` Joerg Roedel
  -1 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2020-01-17  9:59 UTC (permalink / raw)
  To: Lu Baolu; +Cc: ashok.raj, jacob.jun.pan, kevin.tian, iommu, linux-kernel

On Thu, Jan 16, 2020 at 09:52:36AM +0800, Lu Baolu wrote:
> Address field in device TLB invalidation descriptor is qualified
> by the S field. If S field is zero, a single page at page address
> specified by address [63:12] is requested to be invalidated. If S
> field is set, the least significant bit in the address field with
> value 0b (say bit N) indicates the invalidation address range. The
> spec doesn't require the address [N - 1, 0] to be cleared, hence
> remove the unnecessary WARN_ON_ONCE().
> 
> Otherwise, the caller might set "mask = MAX_AGAW_PFN_WIDTH" in order
> to invalidating all the cached mappings on an endpoint, and below
> overflow error will be triggered.
> 
> [...]
> UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1354:3
> shift exponent 64 is too large for 64-bit type 'long long unsigned int'
> [...]
> 
> Reported-and-tested-by: Frank <fgndev@posteo.de>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Does this need a Fixes and/or stable tag?


Regards,

	Joerg


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE()
@ 2020-01-17  9:59   ` Joerg Roedel
  0 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2020-01-17  9:59 UTC (permalink / raw)
  To: Lu Baolu; +Cc: linux-kernel, kevin.tian, ashok.raj, iommu, jacob.jun.pan

On Thu, Jan 16, 2020 at 09:52:36AM +0800, Lu Baolu wrote:
> Address field in device TLB invalidation descriptor is qualified
> by the S field. If S field is zero, a single page at page address
> specified by address [63:12] is requested to be invalidated. If S
> field is set, the least significant bit in the address field with
> value 0b (say bit N) indicates the invalidation address range. The
> spec doesn't require the address [N - 1, 0] to be cleared, hence
> remove the unnecessary WARN_ON_ONCE().
> 
> Otherwise, the caller might set "mask = MAX_AGAW_PFN_WIDTH" in order
> to invalidating all the cached mappings on an endpoint, and below
> overflow error will be triggered.
> 
> [...]
> UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1354:3
> shift exponent 64 is too large for 64-bit type 'long long unsigned int'
> [...]
> 
> Reported-and-tested-by: Frank <fgndev@posteo.de>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Does this need a Fixes and/or stable tag?


Regards,

	Joerg

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE()
  2020-01-17  9:59   ` Joerg Roedel
@ 2020-01-18  2:14     ` Lu Baolu
  -1 siblings, 0 replies; 8+ messages in thread
From: Lu Baolu @ 2020-01-18  2:14 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: baolu.lu, ashok.raj, jacob.jun.pan, kevin.tian, iommu, linux-kernel

Hi Joerg,

On 1/17/20 5:59 PM, Joerg Roedel wrote:
> On Thu, Jan 16, 2020 at 09:52:36AM +0800, Lu Baolu wrote:
>> Address field in device TLB invalidation descriptor is qualified
>> by the S field. If S field is zero, a single page at page address
>> specified by address [63:12] is requested to be invalidated. If S
>> field is set, the least significant bit in the address field with
>> value 0b (say bit N) indicates the invalidation address range. The
>> spec doesn't require the address [N - 1, 0] to be cleared, hence
>> remove the unnecessary WARN_ON_ONCE().
>>
>> Otherwise, the caller might set "mask = MAX_AGAW_PFN_WIDTH" in order
>> to invalidating all the cached mappings on an endpoint, and below
>> overflow error will be triggered.
>>
>> [...]
>> UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1354:3
>> shift exponent 64 is too large for 64-bit type 'long long unsigned int'
>> [...]
>>
>> Reported-and-tested-by: Frank <fgndev@posteo.de>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> 
> Does this need a Fixes and/or stable tag?
> 

This doesn't cause any errors, just an unnecessary checking of

	"0 & ((1UL << 64) - 1)"

in some cases.

> 
> Regards,
> 
> 	Joerg

Best regards,
baolu

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE()
@ 2020-01-18  2:14     ` Lu Baolu
  0 siblings, 0 replies; 8+ messages in thread
From: Lu Baolu @ 2020-01-18  2:14 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: kevin.tian, ashok.raj, linux-kernel, iommu, jacob.jun.pan

Hi Joerg,

On 1/17/20 5:59 PM, Joerg Roedel wrote:
> On Thu, Jan 16, 2020 at 09:52:36AM +0800, Lu Baolu wrote:
>> Address field in device TLB invalidation descriptor is qualified
>> by the S field. If S field is zero, a single page at page address
>> specified by address [63:12] is requested to be invalidated. If S
>> field is set, the least significant bit in the address field with
>> value 0b (say bit N) indicates the invalidation address range. The
>> spec doesn't require the address [N - 1, 0] to be cleared, hence
>> remove the unnecessary WARN_ON_ONCE().
>>
>> Otherwise, the caller might set "mask = MAX_AGAW_PFN_WIDTH" in order
>> to invalidating all the cached mappings on an endpoint, and below
>> overflow error will be triggered.
>>
>> [...]
>> UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1354:3
>> shift exponent 64 is too large for 64-bit type 'long long unsigned int'
>> [...]
>>
>> Reported-and-tested-by: Frank <fgndev@posteo.de>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> 
> Does this need a Fixes and/or stable tag?
> 

This doesn't cause any errors, just an unnecessary checking of

	"0 & ((1UL << 64) - 1)"

in some cases.

> 
> Regards,
> 
> 	Joerg

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE()
  2020-01-18  2:14     ` Lu Baolu
@ 2020-01-24 14:36       ` Joerg Roedel
  -1 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2020-01-24 14:36 UTC (permalink / raw)
  To: Lu Baolu; +Cc: ashok.raj, jacob.jun.pan, kevin.tian, iommu, linux-kernel

On Sat, Jan 18, 2020 at 10:14:11AM +0800, Lu Baolu wrote:
> Hi Joerg,
> 
> On 1/17/20 5:59 PM, Joerg Roedel wrote:
> > On Thu, Jan 16, 2020 at 09:52:36AM +0800, Lu Baolu wrote:
> > > Address field in device TLB invalidation descriptor is qualified
> > > by the S field. If S field is zero, a single page at page address
> > > specified by address [63:12] is requested to be invalidated. If S
> > > field is set, the least significant bit in the address field with
> > > value 0b (say bit N) indicates the invalidation address range. The
> > > spec doesn't require the address [N - 1, 0] to be cleared, hence
> > > remove the unnecessary WARN_ON_ONCE().
> > > 
> > > Otherwise, the caller might set "mask = MAX_AGAW_PFN_WIDTH" in order
> > > to invalidating all the cached mappings on an endpoint, and below
> > > overflow error will be triggered.
> > > 
> > > [...]
> > > UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1354:3
> > > shift exponent 64 is too large for 64-bit type 'long long unsigned int'
> > > [...]
> > > 
> > > Reported-and-tested-by: Frank <fgndev@posteo.de>
> > > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> > 
> > Does this need a Fixes and/or stable tag?
> > 
> 
> This doesn't cause any errors, just an unnecessary checking of
> 
> 	"0 & ((1UL << 64) - 1)"
> 
> in some cases.

Okay, applied for v5.6.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE()
@ 2020-01-24 14:36       ` Joerg Roedel
  0 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2020-01-24 14:36 UTC (permalink / raw)
  To: Lu Baolu; +Cc: linux-kernel, kevin.tian, ashok.raj, iommu, jacob.jun.pan

On Sat, Jan 18, 2020 at 10:14:11AM +0800, Lu Baolu wrote:
> Hi Joerg,
> 
> On 1/17/20 5:59 PM, Joerg Roedel wrote:
> > On Thu, Jan 16, 2020 at 09:52:36AM +0800, Lu Baolu wrote:
> > > Address field in device TLB invalidation descriptor is qualified
> > > by the S field. If S field is zero, a single page at page address
> > > specified by address [63:12] is requested to be invalidated. If S
> > > field is set, the least significant bit in the address field with
> > > value 0b (say bit N) indicates the invalidation address range. The
> > > spec doesn't require the address [N - 1, 0] to be cleared, hence
> > > remove the unnecessary WARN_ON_ONCE().
> > > 
> > > Otherwise, the caller might set "mask = MAX_AGAW_PFN_WIDTH" in order
> > > to invalidating all the cached mappings on an endpoint, and below
> > > overflow error will be triggered.
> > > 
> > > [...]
> > > UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1354:3
> > > shift exponent 64 is too large for 64-bit type 'long long unsigned int'
> > > [...]
> > > 
> > > Reported-and-tested-by: Frank <fgndev@posteo.de>
> > > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> > 
> > Does this need a Fixes and/or stable tag?
> > 
> 
> This doesn't cause any errors, just an unnecessary checking of
> 
> 	"0 & ((1UL << 64) - 1)"
> 
> in some cases.

Okay, applied for v5.6.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-01-24 14:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16  1:52 [PATCH 1/1] iommu/vt-d: Remove unnecessary WARN_ON_ONCE() Lu Baolu
2020-01-16  1:52 ` Lu Baolu
2020-01-17  9:59 ` Joerg Roedel
2020-01-17  9:59   ` Joerg Roedel
2020-01-18  2:14   ` Lu Baolu
2020-01-18  2:14     ` Lu Baolu
2020-01-24 14:36     ` Joerg Roedel
2020-01-24 14:36       ` Joerg Roedel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.