* [PATCH] Revert: "PCI: Add ACS quirk for all Cavium devices"
@ 2017-02-14 16:18 Alex Williamson
2017-02-15 16:34 ` Alex Williamson
0 siblings, 1 reply; 4+ messages in thread
From: Alex Williamson @ 2017-02-14 16:18 UTC (permalink / raw)
To: bhelgaas, linux-pci; +Cc: Tirumalesh Chalamarla, linux-kernel, Manish Jaggi
This reverts commit b404bcfbf035413dcce539c8ba2c9986d220d8ed.
The reverted commit makes no attempt to selectively consider devices,
current or future. Instead, it whitelists the entire PCI vendor ID.
This is a reckless approach as we clearly cannot know whether future
device IDs within this vendor ID are susceptible to peer-to-peer.
Additionally, the comment suggests this quirk is only relevant to
ThunderX, which raises further doubt whether it is appropriate to
apply to the entire vendor ID.
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Manish Jaggi <mjaggi@caviumnetworks.com>
Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com>
---
Please submit a new quirk targeting specific devices
drivers/pci/quirks.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800befa8b8b..449eabb438e2 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4060,19 +4060,6 @@ static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags)
#endif
}
-static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
-{
- /*
- * Cavium devices matching this quirk do not perform peer-to-peer
- * with other functions, allowing masking out these bits as if they
- * were unimplemented in the ACS capability.
- */
- acs_flags &= ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR |
- PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT);
-
- return acs_flags ? 0 : 1;
-}
-
/*
* Many Intel PCH root ports do provide ACS-like features to disable peer
* transactions and validate bus numbers in requests, but do not provide an
@@ -4276,8 +4263,6 @@ static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags)
{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_spt_pch_acs },
{ 0x19a2, 0x710, pci_quirk_mf_endpoint_acs }, /* Emulex BE3-R */
{ 0x10df, 0x720, pci_quirk_mf_endpoint_acs }, /* Emulex Skyhawk-R */
- /* Cavium ThunderX */
- { PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, pci_quirk_cavium_acs },
{ 0 }
};
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Revert: "PCI: Add ACS quirk for all Cavium devices"
2017-02-14 16:18 [PATCH] Revert: "PCI: Add ACS quirk for all Cavium devices" Alex Williamson
@ 2017-02-15 16:34 ` Alex Williamson
2017-02-15 16:42 ` Manish Jaggi
0 siblings, 1 reply; 4+ messages in thread
From: Alex Williamson @ 2017-02-15 16:34 UTC (permalink / raw)
To: bhelgaas, linux-pci; +Cc: Tirumalesh Chalamarla, linux-kernel, Manish Jaggi
On Tue, 14 Feb 2017 09:18:18 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:
> This reverts commit b404bcfbf035413dcce539c8ba2c9986d220d8ed.
>
> The reverted commit makes no attempt to selectively consider devices,
> current or future. Instead, it whitelists the entire PCI vendor ID.
> This is a reckless approach as we clearly cannot know whether future
> device IDs within this vendor ID are susceptible to peer-to-peer.
> Additionally, the comment suggests this quirk is only relevant to
> ThunderX, which raises further doubt whether it is appropriate to
> apply to the entire vendor ID.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Manish Jaggi <mjaggi@caviumnetworks.com>
> Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> ---
>
> Please submit a new quirk targeting specific devices
Sorry, I saw the date on the original commit as January 30, 2016 and
forgot we're now in 2017, so I figured that this had only recently gone
in. Since it's actually been around since 4.6, I'll take a different
approach. Instead of reverting, I'm going to assume that this applies
to ThunderX devices as per the comment. I'll follow-up with a new
patch that only applies this to device IDs matching ThunderX as found
here http://pci-ids.ucw.cz/read/PC/177d Perhaps in the meantime,
someone from Cavium will comment on which subset of those devices this
is actually relevant to. Thanks,
Alex
>
> drivers/pci/quirks.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1800befa8b8b..449eabb438e2 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4060,19 +4060,6 @@ static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags)
> #endif
> }
>
> -static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
> -{
> - /*
> - * Cavium devices matching this quirk do not perform peer-to-peer
> - * with other functions, allowing masking out these bits as if they
> - * were unimplemented in the ACS capability.
> - */
> - acs_flags &= ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR |
> - PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT);
> -
> - return acs_flags ? 0 : 1;
> -}
> -
> /*
> * Many Intel PCH root ports do provide ACS-like features to disable peer
> * transactions and validate bus numbers in requests, but do not provide an
> @@ -4276,8 +4263,6 @@ static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags)
> { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_spt_pch_acs },
> { 0x19a2, 0x710, pci_quirk_mf_endpoint_acs }, /* Emulex BE3-R */
> { 0x10df, 0x720, pci_quirk_mf_endpoint_acs }, /* Emulex Skyhawk-R */
> - /* Cavium ThunderX */
> - { PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, pci_quirk_cavium_acs },
> { 0 }
> };
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Revert: "PCI: Add ACS quirk for all Cavium devices"
2017-02-15 16:34 ` Alex Williamson
@ 2017-02-15 16:42 ` Manish Jaggi
2017-02-15 16:57 ` Alex Williamson
0 siblings, 1 reply; 4+ messages in thread
From: Manish Jaggi @ 2017-02-15 16:42 UTC (permalink / raw)
To: Alex Williamson, bhelgaas, linux-pci; +Cc: Tirumalesh Chalamarla, linux-kernel
Hi Alex,
On 2/15/2017 10:04 PM, Alex Williamson wrote:
> On Tue, 14 Feb 2017 09:18:18 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
>> This reverts commit b404bcfbf035413dcce539c8ba2c9986d220d8ed.
>>
>> The reverted commit makes no attempt to selectively consider devices,
>> current or future. Instead, it whitelists the entire PCI vendor ID.
>> This is a reckless approach as we clearly cannot know whether future
>> device IDs within this vendor ID are susceptible to peer-to-peer.
>> Additionally, the comment suggests this quirk is only relevant to
>> ThunderX, which raises further doubt whether it is appropriate to
>> apply to the entire vendor ID.
>>
>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>> Cc: Manish Jaggi <mjaggi@caviumnetworks.com>
>> Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com>
>> ---
>>
>> Please submit a new quirk targeting specific devices
> Sorry, I saw the date on the original commit as January 30, 2016 and
> forgot we're now in 2017, so I figured that this had only recently gone
> in. Since it's actually been around since 4.6, I'll take a different
> approach. Instead of reverting, I'm going to assume that this applies
> to ThunderX devices as per the comment. I'll follow-up with a new
> patch that only applies this to device IDs matching ThunderX as found
> here http://pci-ids.ucw.cz/read/PC/177d Perhaps in the meantime,
> someone from Cavium will comment on which subset of those devices this
> is actually relevant to.
I will provide another patch with the specific device Ids.
> Thanks,
>
> Alex
>
>> drivers/pci/quirks.c | 15 ---------------
>> 1 file changed, 15 deletions(-)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 1800befa8b8b..449eabb438e2 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -4060,19 +4060,6 @@ static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags)
>> #endif
>> }
>>
>> -static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
>> -{
>> - /*
>> - * Cavium devices matching this quirk do not perform peer-to-peer
>> - * with other functions, allowing masking out these bits as if they
>> - * were unimplemented in the ACS capability.
>> - */
>> - acs_flags &= ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR |
>> - PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT);
>> -
>> - return acs_flags ? 0 : 1;
>> -}
>> -
>> /*
>> * Many Intel PCH root ports do provide ACS-like features to disable peer
>> * transactions and validate bus numbers in requests, but do not provide an
>> @@ -4276,8 +4263,6 @@ static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags)
>> { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_spt_pch_acs },
>> { 0x19a2, 0x710, pci_quirk_mf_endpoint_acs }, /* Emulex BE3-R */
>> { 0x10df, 0x720, pci_quirk_mf_endpoint_acs }, /* Emulex Skyhawk-R */
>> - /* Cavium ThunderX */
>> - { PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, pci_quirk_cavium_acs },
>> { 0 }
>> };
>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Revert: "PCI: Add ACS quirk for all Cavium devices"
2017-02-15 16:42 ` Manish Jaggi
@ 2017-02-15 16:57 ` Alex Williamson
0 siblings, 0 replies; 4+ messages in thread
From: Alex Williamson @ 2017-02-15 16:57 UTC (permalink / raw)
To: Manish Jaggi; +Cc: bhelgaas, linux-pci, Tirumalesh Chalamarla, linux-kernel
On Wed, 15 Feb 2017 22:12:54 +0530
Manish Jaggi <mjaggi@caviumnetworks.com> wrote:
> Hi Alex,
>
> On 2/15/2017 10:04 PM, Alex Williamson wrote:
> > On Tue, 14 Feb 2017 09:18:18 -0700
> > Alex Williamson <alex.williamson@redhat.com> wrote:
> >
> >> This reverts commit b404bcfbf035413dcce539c8ba2c9986d220d8ed.
> >>
> >> The reverted commit makes no attempt to selectively consider devices,
> >> current or future. Instead, it whitelists the entire PCI vendor ID.
> >> This is a reckless approach as we clearly cannot know whether future
> >> device IDs within this vendor ID are susceptible to peer-to-peer.
> >> Additionally, the comment suggests this quirk is only relevant to
> >> ThunderX, which raises further doubt whether it is appropriate to
> >> apply to the entire vendor ID.
> >>
> >> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> >> Cc: Manish Jaggi <mjaggi@caviumnetworks.com>
> >> Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> >> ---
> >>
> >> Please submit a new quirk targeting specific devices
> > Sorry, I saw the date on the original commit as January 30, 2016 and
> > forgot we're now in 2017, so I figured that this had only recently gone
> > in. Since it's actually been around since 4.6, I'll take a different
> > approach. Instead of reverting, I'm going to assume that this applies
> > to ThunderX devices as per the comment. I'll follow-up with a new
> > patch that only applies this to device IDs matching ThunderX as found
> > here http://pci-ids.ucw.cz/read/PC/177d Perhaps in the meantime,
> > someone from Cavium will comment on which subset of those devices this
> > is actually relevant to.
> I will provide another patch with the specific device Ids.
Perfect, thank you Manish.
Alex
> >> drivers/pci/quirks.c | 15 ---------------
> >> 1 file changed, 15 deletions(-)
> >>
> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> >> index 1800befa8b8b..449eabb438e2 100644
> >> --- a/drivers/pci/quirks.c
> >> +++ b/drivers/pci/quirks.c
> >> @@ -4060,19 +4060,6 @@ static int pci_quirk_amd_sb_acs(struct pci_dev *dev, u16 acs_flags)
> >> #endif
> >> }
> >>
> >> -static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
> >> -{
> >> - /*
> >> - * Cavium devices matching this quirk do not perform peer-to-peer
> >> - * with other functions, allowing masking out these bits as if they
> >> - * were unimplemented in the ACS capability.
> >> - */
> >> - acs_flags &= ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR |
> >> - PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_DT);
> >> -
> >> - return acs_flags ? 0 : 1;
> >> -}
> >> -
> >> /*
> >> * Many Intel PCH root ports do provide ACS-like features to disable peer
> >> * transactions and validate bus numbers in requests, but do not provide an
> >> @@ -4276,8 +4263,6 @@ static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev, u16 acs_flags)
> >> { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_spt_pch_acs },
> >> { 0x19a2, 0x710, pci_quirk_mf_endpoint_acs }, /* Emulex BE3-R */
> >> { 0x10df, 0x720, pci_quirk_mf_endpoint_acs }, /* Emulex Skyhawk-R */
> >> - /* Cavium ThunderX */
> >> - { PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID, pci_quirk_cavium_acs },
> >> { 0 }
> >> };
> >>
> >>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-02-15 16:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-14 16:18 [PATCH] Revert: "PCI: Add ACS quirk for all Cavium devices" Alex Williamson
2017-02-15 16:34 ` Alex Williamson
2017-02-15 16:42 ` Manish Jaggi
2017-02-15 16:57 ` Alex Williamson
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).