答复: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-09 Thread Jim,Yan
Hi Baolu,

> -邮件原件-
> 发件人: Lu Baolu [mailto:baolu...@linux.intel.com]
> 发送时间: 2020年1月9日 16:53
> 收件人: Christoph Hellwig 
> 抄送: baolu...@linux.intel.com; Joerg Roedel ; Roland
> Dreier ; Jim,Yan ;
> iommu@lists.linux-foundation.org
> 主题: Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched
> devices
> 
> On 1/9/20 3:06 PM, Christoph Hellwig wrote:
> > On Thu, Jan 09, 2020 at 07:28:41AM +0800, Lu Baolu wrote:
> >> This patch has been replaced with this one.
> >>
> >> https://lkml.org/lkml/2020/1/5/103
> >
> > That still mentions a "nvme host device", which despite the different
> > spelling still doesn't make any sense.
> >
> 
> Jim, can you please refine it accordingly?
> 
> Best regards,
> Baolu


Yes, I am working on it.

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

Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-09 Thread Lu Baolu

On 1/9/20 3:06 PM, Christoph Hellwig wrote:

On Thu, Jan 09, 2020 at 07:28:41AM +0800, Lu Baolu wrote:

This patch has been replaced with this one.

https://lkml.org/lkml/2020/1/5/103


That still mentions a "nvme host device", which despite the different
spelling still doesn't make any sense.



Jim, can you please refine it accordingly?

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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-08 Thread Christoph Hellwig
On Thu, Jan 09, 2020 at 07:28:41AM +0800, Lu Baolu wrote:
> This patch has been replaced with this one.
> 
> https://lkml.org/lkml/2020/1/5/103

That still mentions a "nvme host device", which despite the different
spelling still doesn't make any sense.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-08 Thread Roland Dreier via iommu
> Are you willing to add your reviewed-by for Jim's v2 patch? I will
> queue it for v5.6 if you both agree.

Sure:

Reviewed-by: Roland Dreier 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-08 Thread Lu Baolu

Hi Christoph,

On 1/8/20 10:16 PM, Christoph Hellwig wrote:

+/*
+ * We expect devices with endpoint scope to have normal PCI
+ * headers, and devices with bridge scope to have bridge PCI
+ * headers.  However some PCI devices may be listed in the
+ * DMAR table with bridge scope, even though they have a
+ * normal PCI header. We don't declare a socpe mismatch for
+ * below special cases.
+ */


Please use up all 80 lines for comments.


+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2f0d,  /* NTB devices  */
+quirk_dmar_scope_mismatch);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2020,  /* NVME host */
+quirk_dmar_scope_mismatch);


As said before "NVME host" host.  Besides the wrong spelling of NVMe,
the NVMe host is the Linux kernel, so describing a device as such seems
rather bogus.



This patch has been replaced with this one.

https://lkml.org/lkml/2020/1/5/103

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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-08 Thread Christoph Hellwig
> +/*
> + * We expect devices with endpoint scope to have normal PCI
> + * headers, and devices with bridge scope to have bridge PCI
> + * headers.  However some PCI devices may be listed in the
> + * DMAR table with bridge scope, even though they have a
> + * normal PCI header. We don't declare a socpe mismatch for
> + * below special cases.
> + */

Please use up all 80 lines for comments.

> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2f0d,/* NTB devices  
> */
> +  quirk_dmar_scope_mismatch);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2020,/* NVME host */
> +  quirk_dmar_scope_mismatch);

As said before "NVME host" host.  Besides the wrong spelling of NVMe,
the NVMe host is the Linux kernel, so describing a device as such seems
rather bogus.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-06 Thread Lu Baolu

Hi,

On 1/7/20 9:30 AM, Jerry Snitselaar wrote:

On Tue Jan 07 20, Lu Baolu wrote:

Hi Jerry,

On 1/7/20 1:05 AM, Jerry Snitselaar wrote:

On Wed Jan 01 20, Roland Dreier via iommu wrote:
We saw more devices with the same mismatch quirk. So maintaining 
them in

a quirk table will make it more readable and maintainable.


I guess I disagree about the maintainable part, given that this patch
already regresses Broadwell NTB.

I'm not even sure what the DMAR table says about NTB on my Skylake
systems, exactly because the existing code means I did not have any
problems.  But we might need to add device 201Ch too.

Maybe we don't need the mismatch check at all?  Your patch sets the
quirk if any possibly mismatching device is present in the system, so
we'll ignore any scope mismatch on a system with, say, the 8086:2020
NVMe host in it.  So could we just drop the check completely and not
have a quirk to disable the check?

- R.


If the check is removed what happens for cases where there is an actual
problem in the dmar table? I just worked an issue with some Intel
people where a purley system had an rmrr entry pointing to a bridge as
the endpoint device instead of the raid module sitting behind it.


The latest solution was here. https://lkml.org/lkml/2020/1/5/103, does
this work for you?

Best regards,
baolu



Hi Baolu,

They resolved it by updating the rmrr entry in the dmar table to add
the extra path needed for it to point at the raid module. Looking
at the code though I imagine without the firmware update they would
still have the problem because IIRC it was a combo of an endpoint
scope type, and a pci bridge header so that first check would fail
as it did before. My worry was if the suggestion is to remove the
check completely, a case like that wouldn't report anything wrong.


Yes, agreed.



Jim's latest patch I think solves the issue for what he was seeing
and the NTB case.



Jerry and Roland,

Are you willing to add your reviewed-by for Jim's v2 patch? I will
queue it for v5.6 if you both agree.

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

Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-06 Thread Jerry Snitselaar

On Tue Jan 07 20, Lu Baolu wrote:

Hi Jerry,

On 1/7/20 1:05 AM, Jerry Snitselaar wrote:

On Wed Jan 01 20, Roland Dreier via iommu wrote:

We saw more devices with the same mismatch quirk. So maintaining them in
a quirk table will make it more readable and maintainable.


I guess I disagree about the maintainable part, given that this patch
already regresses Broadwell NTB.

I'm not even sure what the DMAR table says about NTB on my Skylake
systems, exactly because the existing code means I did not have any
problems.  But we might need to add device 201Ch too.

Maybe we don't need the mismatch check at all?  Your patch sets the
quirk if any possibly mismatching device is present in the system, so
we'll ignore any scope mismatch on a system with, say, the 8086:2020
NVMe host in it.  So could we just drop the check completely and not
have a quirk to disable the check?

- R.


If the check is removed what happens for cases where there is an actual
problem in the dmar table? I just worked an issue with some Intel
people where a purley system had an rmrr entry pointing to a bridge as
the endpoint device instead of the raid module sitting behind it.


The latest solution was here. https://lkml.org/lkml/2020/1/5/103, does
this work for you?

Best regards,
baolu



Hi Baolu,

They resolved it by updating the rmrr entry in the dmar table to add
the extra path needed for it to point at the raid module. Looking
at the code though I imagine without the firmware update they would
still have the problem because IIRC it was a combo of an endpoint
scope type, and a pci bridge header so that first check would fail
as it did before. My worry was if the suggestion is to remove the
check completely, a case like that wouldn't report anything wrong.

Jim's latest patch I think solves the issue for what he was seeing
and the NTB case.

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

Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-06 Thread Lu Baolu

Hi Jerry,

On 1/7/20 1:05 AM, Jerry Snitselaar wrote:

On Wed Jan 01 20, Roland Dreier via iommu wrote:

We saw more devices with the same mismatch quirk. So maintaining them in
a quirk table will make it more readable and maintainable.


I guess I disagree about the maintainable part, given that this patch
already regresses Broadwell NTB.

I'm not even sure what the DMAR table says about NTB on my Skylake
systems, exactly because the existing code means I did not have any
problems.  But we might need to add device 201Ch too.

Maybe we don't need the mismatch check at all?  Your patch sets the
quirk if any possibly mismatching device is present in the system, so
we'll ignore any scope mismatch on a system with, say, the 8086:2020
NVMe host in it.  So could we just drop the check completely and not
have a quirk to disable the check?

- R.


If the check is removed what happens for cases where there is an actual
problem in the dmar table? I just worked an issue with some Intel
people where a purley system had an rmrr entry pointing to a bridge as
the endpoint device instead of the raid module sitting behind it.


The latest solution was here. https://lkml.org/lkml/2020/1/5/103, does
this work for you?

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

Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-06 Thread Jerry Snitselaar

On Wed Jan 01 20, Roland Dreier via iommu wrote:

We saw more devices with the same mismatch quirk. So maintaining them in
a quirk table will make it more readable and maintainable.


I guess I disagree about the maintainable part, given that this patch
already regresses Broadwell NTB.

I'm not even sure what the DMAR table says about NTB on my Skylake
systems, exactly because the existing code means I did not have any
problems.  But we might need to add device 201Ch too.

Maybe we don't need the mismatch check at all?  Your patch sets the
quirk if any possibly mismatching device is present in the system, so
we'll ignore any scope mismatch on a system with, say, the 8086:2020
NVMe host in it.  So could we just drop the check completely and not
have a quirk to disable the check?

- R.


If the check is removed what happens for cases where there is an actual
problem in the dmar table? I just worked an issue with some Intel
people where a purley system had an rmrr entry pointing to a bridge as
the endpoint device instead of the raid module sitting behind it.


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



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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-04 Thread Lu Baolu

Hi Jim,

On 1/5/20 12:52 AM, Roland Dreier wrote:

Jim proposed another solution.

https://lkml.org/lkml/2019/12/23/653

Does this work for you?


Yes, that's OK for the cases I've seen too.  All the NTB devices I've
seen are PCI_CLASS_BRIDGE_OTHER with type 0 headers, so this patch
would not break anything.  And I think the idea of allowing DMAR
bridge scope for all devices with PCI class bridge is logical - BIOS
writers probably are going by PCI class rather than header type when
assigning scope.


Can you please post a v2 of this patch with the change you proposed in
https://lkml.org/lkml/2019/12/23/653?

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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-04 Thread Roland Dreier via iommu
> Jim proposed another solution.
>
> https://lkml.org/lkml/2019/12/23/653
>
> Does this work for you?

Yes, that's OK for the cases I've seen too.  All the NTB devices I've
seen are PCI_CLASS_BRIDGE_OTHER with type 0 headers, so this patch
would not break anything.  And I think the idea of allowing DMAR
bridge scope for all devices with PCI class bridge is logical - BIOS
writers probably are going by PCI class rather than header type when
assigning scope.

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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-02 Thread Lu Baolu

Hi Roland,

Jim proposed another solution.

https://lkml.org/lkml/2019/12/23/653

Does this work for you?

Best regards,
baolu

On 1/2/20 10:25 AM, Roland Dreier wrote:

We saw more devices with the same mismatch quirk. So maintaining them in
a quirk table will make it more readable and maintainable.


I guess I disagree about the maintainable part, given that this patch
already regresses Broadwell NTB.

I'm not even sure what the DMAR table says about NTB on my Skylake
systems, exactly because the existing code means I did not have any
problems.  But we might need to add device 201Ch too.

Maybe we don't need the mismatch check at all?  Your patch sets the
quirk if any possibly mismatching device is present in the system, so
we'll ignore any scope mismatch on a system with, say, the 8086:2020
NVMe host in it.  So could we just drop the check completely and not
have a quirk to disable the check?

  - R.


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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-01 Thread Lu Baolu

Hi,

On 1/2/20 10:25 AM, Roland Dreier wrote:

We saw more devices with the same mismatch quirk. So maintaining them in
a quirk table will make it more readable and maintainable.


I guess I disagree about the maintainable part, given that this patch
already regresses Broadwell NTB.

I'm not even sure what the DMAR table says about NTB on my Skylake
systems, exactly because the existing code means I did not have any
problems.  But we might need to add device 201Ch too.

Maybe we don't need the mismatch check at all?  Your patch sets the
quirk if any possibly mismatching device is present in the system, so
we'll ignore any scope mismatch on a system with, say, the 8086:2020
NVMe host in it.  So could we just drop the check completely and not
have a quirk to disable the check?


Fair enough. Instead of no check, how about putting a pr_info() there
and give end user a chance to know this?

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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-01 Thread Roland Dreier via iommu
> We saw more devices with the same mismatch quirk. So maintaining them in
> a quirk table will make it more readable and maintainable.

I guess I disagree about the maintainable part, given that this patch
already regresses Broadwell NTB.

I'm not even sure what the DMAR table says about NTB on my Skylake
systems, exactly because the existing code means I did not have any
problems.  But we might need to add device 201Ch too.

Maybe we don't need the mismatch check at all?  Your patch sets the
quirk if any possibly mismatching device is present in the system, so
we'll ignore any scope mismatch on a system with, say, the 8086:2020
NVMe host in it.  So could we just drop the check completely and not
have a quirk to disable the check?

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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-01 Thread Roland Dreier via iommu
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2f0d,  /* NTB devices  */
> +quirk_dmar_scope_mismatch);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2020,  /* NVME host */
> +quirk_dmar_scope_mismatch);

what's the motivation for changing the logic into a quirk table, which
has to be maintained with new device IDs?

In particular this has the Haswell NTB ID 2F0Dh but already leaves out
the Broadwell ID 6F0Dh.

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


Re: [PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-01 Thread Lu Baolu

Hi,

On 1/2/20 10:11 AM, Roland Dreier wrote:

+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2f0d,  /* NTB devices  */
+quirk_dmar_scope_mismatch);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2020,  /* NVME host */
+quirk_dmar_scope_mismatch);


what's the motivation for changing the logic into a quirk table, which
has to be maintained with new device IDs?


We saw more devices with the same mismatch quirk. So maintaining them in
a quirk table will make it more readable and maintainable.

Best regards,
-baolu



In particular this has the Haswell NTB ID 2F0Dh but already leaves out
the Broadwell ID 6F0Dh.

  - R.


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


[PATCH 22/22] iommu/vt-d: Add a quirk flag for scope mismatched devices

2020-01-01 Thread Lu Baolu
We expect devices with endpoint scope to have normal PCI headers,
and devices with bridge scope to have bridge PCI headers. However,
some PCI devices may be listed in the DMAR table with bridge scope,
even though they have a normal PCI header. Add a quirk flag for
those special devices.

Cc: Roland Dreier 
Cc: Jim Yan 
Signed-off-by: Lu Baolu 
Reviewed-by: Jerry Snitselaar 
Tested-by: Jim Yan 
---
 drivers/iommu/dmar.c | 37 +++--
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index fb30d5053664..fc24abc70a05 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -65,6 +65,26 @@ static void free_iommu(struct intel_iommu *iommu);
 
 extern const struct iommu_ops intel_iommu_ops;
 
+static int scope_mismatch_quirk;
+static void quirk_dmar_scope_mismatch(struct pci_dev *dev)
+{
+   pci_info(dev, "scope mismatch ignored\n");
+   scope_mismatch_quirk = 1;
+}
+
+/*
+ * We expect devices with endpoint scope to have normal PCI
+ * headers, and devices with bridge scope to have bridge PCI
+ * headers.  However some PCI devices may be listed in the
+ * DMAR table with bridge scope, even though they have a
+ * normal PCI header. We don't declare a socpe mismatch for
+ * below special cases.
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2f0d,  /* NTB devices  */
+quirk_dmar_scope_mismatch);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2020,  /* NVME host */
+quirk_dmar_scope_mismatch);
+
 static void dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
 {
/*
@@ -231,20 +251,9 @@ int dmar_insert_dev_scope(struct dmar_pci_notify_info 
*info,
if (!dmar_match_pci_path(info, scope->bus, path, level))
continue;
 
-   /*
-* We expect devices with endpoint scope to have normal PCI
-* headers, and devices with bridge scope to have bridge PCI
-* headers.  However PCI NTB devices may be listed in the
-* DMAR table with bridge scope, even though they have a
-* normal PCI header.  NTB devices are identified by class
-* "BRIDGE_OTHER" (0680h) - we don't declare a socpe mismatch
-* for this special case.
-*/
-   if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT &&
-info->dev->hdr_type != PCI_HEADER_TYPE_NORMAL) ||
-   (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE &&
-(info->dev->hdr_type == PCI_HEADER_TYPE_NORMAL &&
- info->dev->class >> 8 != PCI_CLASS_BRIDGE_OTHER))) {
+   if (!scope_mismatch_quirk &&
+   ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) ^
+(info->dev->hdr_type == PCI_HEADER_TYPE_NORMAL))) {
pr_warn("Device scope type does not match for %s\n",
pci_name(info->dev));
return -EINVAL;
-- 
2.17.1

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