* [PATCH 0/2] Thunderbolt: allow vendor ID override for NVM programming @ 2021-11-24 16:34 Francisco Blas Izquierdo Riera (klondike) 2021-11-24 16:37 ` [PATCH 1/2] thunderbolt: " Francisco Blas Izquierdo Riera (klondike) 2021-11-24 16:37 ` [PATCH 2/2] Documentation: explain how to override Thunderbolt Vendor ID Francisco Blas Izquierdo Riera (klondike) 0 siblings, 2 replies; 9+ messages in thread From: Francisco Blas Izquierdo Riera (klondike) @ 2021-11-24 16:34 UTC (permalink / raw) To: linux-usb Cc: Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Greg Kroah-Hartman, Lukas Wunner, Jonathan Corbet, linux-doc, Francisco Blas Izquierdo Riera (klondike) Currently, the vendor ID reported by the chipset is checked before to avoid accidentally programming devices from unsupported vendors with a different NVM structure. Certain Thunderbolt devices store the vendor ID in the NVM, therefore if the NVM has become corrrupted the device will report an invalid vendor ID and reflashing will be impossible on GNU/Linux even if the device can boot in safe mode. Such devices can still be programmed just fine if the vendor ID check is overridden. Nevertheless, overriding these checks introduces the risk of damaging controllers from other manufacturers which is a clearly undesirable result. Instead we allow the user to make a concious choice to override the vendor ID by passing it as a parameter to this module. Currently, this is done by expanding the condition to validate the vendor ID to also check this parameter's value but, if new NVM structures are added and a choice has to be made, the code should prefer this parameter over the harrdware reported one when making the choice. This patch also updates the Thunderbolt documentation to explain how this parameter works. root (2): thunderbolt: allow vendor ID override for NVM programming Documentation: explain how to override Thunderbolt Vendor ID Documentation/admin-guide/thunderbolt.rst | 10 ++++++++++ drivers/thunderbolt/switch.c | 9 ++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] thunderbolt: allow vendor ID override for NVM programming 2021-11-24 16:34 [PATCH 0/2] Thunderbolt: allow vendor ID override for NVM programming Francisco Blas Izquierdo Riera (klondike) @ 2021-11-24 16:37 ` Francisco Blas Izquierdo Riera (klondike) 2021-11-24 18:26 ` Greg Kroah-Hartman 2021-11-25 6:16 ` Mika Westerberg 2021-11-24 16:37 ` [PATCH 2/2] Documentation: explain how to override Thunderbolt Vendor ID Francisco Blas Izquierdo Riera (klondike) 1 sibling, 2 replies; 9+ messages in thread From: Francisco Blas Izquierdo Riera (klondike) @ 2021-11-24 16:37 UTC (permalink / raw) To: linux-usb Cc: Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Greg Kroah-Hartman, Lukas Wunner, Jonathan Corbet, linux-doc, Francisco Blas Izquierdo Riera (klondike) Currently, the vendor ID reported by the chipset is checked before to avoid accidentally programming devices from unsupported vendors with a different NVM structure. Certain Thunderbolt devices store the vendor ID in the NVM, therefore if the NVM has become corrrupted the device will report an invalid vendor ID and reflashing will be impossible on GNU/Linux even if the device can boot in safe mode. This patch adds a new parameter ``switch_nvm_vendor_override`` which can be used to override the vendor ID used for detecting the NVM structure allowing to reflash (and authenticate) a new, valid image on the device. Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@klondike.es> --- drivers/thunderbolt/switch.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index 3014146081..a7959c3f3f 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -13,6 +13,7 @@ #include <linux/sched/signal.h> #include <linux/sizes.h> #include <linux/slab.h> +#include <linux/moduleparam.h> #include "tb.h" @@ -34,6 +35,10 @@ struct nvm_auth_status { static LIST_HEAD(nvm_auth_status_cache); static DEFINE_MUTEX(nvm_auth_status_lock); +static short switch_nvm_vendor_override = -1; +module_param(switch_nvm_vendor_override, short, 0440); +MODULE_PARM_DESC(switch_nvm_vendor_override, "Override the switch vendor id on the nvm access routines"); + static struct nvm_auth_status *__nvm_get_auth_status(const struct tb_switch *sw) { struct nvm_auth_status *st; @@ -391,7 +396,9 @@ static int tb_switch_nvm_add(struct tb_switch *sw) * relax this in the future when we learn other NVM formats. */ if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL && - sw->config.vendor_id != 0x8087) { + sw->config.vendor_id != 0x8087 && + switch_nvm_vendor_override != PCI_VENDOR_ID_INTEL && + switch_nvm_vendor_override != 0x8087) { dev_info(&sw->dev, "NVM format of vendor %#x is not known, disabling NVM upgrade\n", sw->config.vendor_id); ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] thunderbolt: allow vendor ID override for NVM programming 2021-11-24 16:37 ` [PATCH 1/2] thunderbolt: " Francisco Blas Izquierdo Riera (klondike) @ 2021-11-24 18:26 ` Greg Kroah-Hartman 2021-11-24 18:32 ` klondike 2021-11-25 6:16 ` Mika Westerberg 1 sibling, 1 reply; 9+ messages in thread From: Greg Kroah-Hartman @ 2021-11-24 18:26 UTC (permalink / raw) To: Francisco Blas Izquierdo Riera (klondike) Cc: linux-usb, Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Lukas Wunner, Jonathan Corbet, linux-doc On Wed, Nov 24, 2021 at 05:37:05PM +0100, Francisco Blas Izquierdo Riera (klondike) wrote: > Currently, the vendor ID reported by the chipset is checked before to > avoid accidentally programming devices from unsupported vendors with > a different NVM structure. > > Certain Thunderbolt devices store the vendor ID in the NVM, therefore > if the NVM has become corrrupted the device will report an invalid > vendor ID and reflashing will be impossible on GNU/Linux even if the > device can boot in safe mode. > > This patch adds a new parameter ``switch_nvm_vendor_override`` which > can be used to override the vendor ID used for detecting the NVM > structure allowing to reflash (and authenticate) a new, valid > image on the device. > > Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@klondike.es> > --- > drivers/thunderbolt/switch.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > index 3014146081..a7959c3f3f 100644 > --- a/drivers/thunderbolt/switch.c > +++ b/drivers/thunderbolt/switch.c > @@ -13,6 +13,7 @@ > #include <linux/sched/signal.h> > #include <linux/sizes.h> > #include <linux/slab.h> > +#include <linux/moduleparam.h> > #include "tb.h" > @@ -34,6 +35,10 @@ struct nvm_auth_status { > static LIST_HEAD(nvm_auth_status_cache); > static DEFINE_MUTEX(nvm_auth_status_lock); > +static short switch_nvm_vendor_override = -1; > +module_param(switch_nvm_vendor_override, short, 0440); > +MODULE_PARM_DESC(switch_nvm_vendor_override, "Override the switch vendor id on the nvm access routines"); > + > static struct nvm_auth_status *__nvm_get_auth_status(const struct tb_switch *sw) > { > struct nvm_auth_status *st; > @@ -391,7 +396,9 @@ static int tb_switch_nvm_add(struct tb_switch *sw) > * relax this in the future when we learn other NVM formats. > */ > if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL && > - sw->config.vendor_id != 0x8087) { > + sw->config.vendor_id != 0x8087 && > + switch_nvm_vendor_override != PCI_VENDOR_ID_INTEL && > + switch_nvm_vendor_override != 0x8087) { > dev_info(&sw->dev, > "NVM format of vendor %#x is not known, disabling NVM upgrade\n", > sw->config.vendor_id); Patch is corrupted :( Anyway, module parameters are from the 1990's and should stay there. Please use a per-device way to handle this instead, as trying to handle module parameters is very difficult over time. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] thunderbolt: allow vendor ID override for NVM programming 2021-11-24 18:26 ` Greg Kroah-Hartman @ 2021-11-24 18:32 ` klondike 2021-11-24 18:37 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ messages in thread From: klondike @ 2021-11-24 18:32 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-usb, Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Lukas Wunner, Jonathan Corbet, linux-doc [-- Attachment #1.1.1: Type: text/plain, Size: 2983 bytes --] El 24/11/21 a las 19:26, Greg Kroah-Hartman escribió: > On Wed, Nov 24, 2021 at 05:37:05PM +0100, Francisco Blas Izquierdo Riera (klondike) wrote: >> Currently, the vendor ID reported by the chipset is checked before to >> avoid accidentally programming devices from unsupported vendors with >> a different NVM structure. >> >> Certain Thunderbolt devices store the vendor ID in the NVM, therefore >> if the NVM has become corrrupted the device will report an invalid >> vendor ID and reflashing will be impossible on GNU/Linux even if the >> device can boot in safe mode. >> >> This patch adds a new parameter ``switch_nvm_vendor_override`` which >> can be used to override the vendor ID used for detecting the NVM >> structure allowing to reflash (and authenticate) a new, valid >> image on the device. >> >> Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@klondike.es> >> --- >> drivers/thunderbolt/switch.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c >> index 3014146081..a7959c3f3f 100644 >> --- a/drivers/thunderbolt/switch.c >> +++ b/drivers/thunderbolt/switch.c >> @@ -13,6 +13,7 @@ >> #include <linux/sched/signal.h> >> #include <linux/sizes.h> >> #include <linux/slab.h> >> +#include <linux/moduleparam.h> >> #include "tb.h" >> @@ -34,6 +35,10 @@ struct nvm_auth_status { >> static LIST_HEAD(nvm_auth_status_cache); >> static DEFINE_MUTEX(nvm_auth_status_lock); >> +static short switch_nvm_vendor_override = -1; >> +module_param(switch_nvm_vendor_override, short, 0440); >> +MODULE_PARM_DESC(switch_nvm_vendor_override, "Override the switch vendor id on the nvm access routines"); >> + >> static struct nvm_auth_status *__nvm_get_auth_status(const struct tb_switch *sw) >> { >> struct nvm_auth_status *st; >> @@ -391,7 +396,9 @@ static int tb_switch_nvm_add(struct tb_switch *sw) >> * relax this in the future when we learn other NVM formats. >> */ >> if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL && >> - sw->config.vendor_id != 0x8087) { >> + sw->config.vendor_id != 0x8087 && >> + switch_nvm_vendor_override != PCI_VENDOR_ID_INTEL && >> + switch_nvm_vendor_override != 0x8087) { >> dev_info(&sw->dev, >> "NVM format of vendor %#x is not known, disabling NVM upgrade\n", >> sw->config.vendor_id); > Patch is corrupted :( > > Anyway, module parameters are from the 1990's and should stay there. > Please use a per-device way to handle this instead, as trying to handle > module parameters is very difficult over time. > > thanks, > > greg k-h Hi Greg! Thanks for your feedback. I'm a bit uncertain about what you mean with a per-device way. Do you mean through the sysfs interface? If so how to do so on device discovery time (which is what the Thunderbolt driver does)? I'm surely missing something here so I would really appreciate a pointer in the right direction. Francisco [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 128291 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] thunderbolt: allow vendor ID override for NVM programming 2021-11-24 18:32 ` klondike @ 2021-11-24 18:37 ` Greg Kroah-Hartman 2021-11-24 18:56 ` klondike 0 siblings, 1 reply; 9+ messages in thread From: Greg Kroah-Hartman @ 2021-11-24 18:37 UTC (permalink / raw) To: klondike Cc: linux-usb, Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Lukas Wunner, Jonathan Corbet, linux-doc On Wed, Nov 24, 2021 at 07:32:29PM +0100, klondike wrote: > El 24/11/21 a las 19:26, Greg Kroah-Hartman escribió: > > On Wed, Nov 24, 2021 at 05:37:05PM +0100, Francisco Blas Izquierdo Riera (klondike) wrote: > >> Currently, the vendor ID reported by the chipset is checked before to > >> avoid accidentally programming devices from unsupported vendors with > >> a different NVM structure. > >> > >> Certain Thunderbolt devices store the vendor ID in the NVM, therefore > >> if the NVM has become corrrupted the device will report an invalid > >> vendor ID and reflashing will be impossible on GNU/Linux even if the > >> device can boot in safe mode. > >> > >> This patch adds a new parameter ``switch_nvm_vendor_override`` which > >> can be used to override the vendor ID used for detecting the NVM > >> structure allowing to reflash (and authenticate) a new, valid > >> image on the device. > >> > >> Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@klondike.es> > >> --- > >> drivers/thunderbolt/switch.c | 9 ++++++++- > >> 1 file changed, 8 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > >> index 3014146081..a7959c3f3f 100644 > >> --- a/drivers/thunderbolt/switch.c > >> +++ b/drivers/thunderbolt/switch.c > >> @@ -13,6 +13,7 @@ > >> #include <linux/sched/signal.h> > >> #include <linux/sizes.h> > >> #include <linux/slab.h> > >> +#include <linux/moduleparam.h> > >> #include "tb.h" > >> @@ -34,6 +35,10 @@ struct nvm_auth_status { > >> static LIST_HEAD(nvm_auth_status_cache); > >> static DEFINE_MUTEX(nvm_auth_status_lock); > >> +static short switch_nvm_vendor_override = -1; > >> +module_param(switch_nvm_vendor_override, short, 0440); > >> +MODULE_PARM_DESC(switch_nvm_vendor_override, "Override the switch vendor id on the nvm access routines"); > >> + > >> static struct nvm_auth_status *__nvm_get_auth_status(const struct tb_switch *sw) > >> { > >> struct nvm_auth_status *st; > >> @@ -391,7 +396,9 @@ static int tb_switch_nvm_add(struct tb_switch *sw) > >> * relax this in the future when we learn other NVM formats. > >> */ > >> if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL && > >> - sw->config.vendor_id != 0x8087) { > >> + sw->config.vendor_id != 0x8087 && > >> + switch_nvm_vendor_override != PCI_VENDOR_ID_INTEL && > >> + switch_nvm_vendor_override != 0x8087) { > >> dev_info(&sw->dev, > >> "NVM format of vendor %#x is not known, disabling NVM upgrade\n", > >> sw->config.vendor_id); > > Patch is corrupted :( > > > > Anyway, module parameters are from the 1990's and should stay there. > > Please use a per-device way to handle this instead, as trying to handle > > module parameters is very difficult over time. > > > > thanks, > > > > greg k-h > > Hi Greg! > > Thanks for your feedback. I'm a bit uncertain about what you mean with > a per-device way. Do you mean through the sysfs interface? If so how > to do so on device discovery time (which is what the Thunderbolt > driver does)? > > I'm surely missing something here so I would really appreciate a > pointer in the right direction. Ah, forgot about discovery time, you want this before the device is probed... Then what about the existing "new_id" file for the driver? That's what it is there for, right? thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] thunderbolt: allow vendor ID override for NVM programming 2021-11-24 18:37 ` Greg Kroah-Hartman @ 2021-11-24 18:56 ` klondike 2021-11-25 6:02 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ messages in thread From: klondike @ 2021-11-24 18:56 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-usb, Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Lukas Wunner, Jonathan Corbet, linux-doc [-- Attachment #1.1.1: Type: text/plain, Size: 3978 bytes --] El 24/11/21 a las 19:37, Greg Kroah-Hartman escribió: > On Wed, Nov 24, 2021 at 07:32:29PM +0100, klondike wrote: >> El 24/11/21 a las 19:26, Greg Kroah-Hartman escribió: >>> On Wed, Nov 24, 2021 at 05:37:05PM +0100, Francisco Blas Izquierdo Riera (klondike) wrote: >>>> Currently, the vendor ID reported by the chipset is checked before to >>>> avoid accidentally programming devices from unsupported vendors with >>>> a different NVM structure. >>>> >>>> Certain Thunderbolt devices store the vendor ID in the NVM, therefore >>>> if the NVM has become corrrupted the device will report an invalid >>>> vendor ID and reflashing will be impossible on GNU/Linux even if the >>>> device can boot in safe mode. >>>> >>>> This patch adds a new parameter ``switch_nvm_vendor_override`` which >>>> can be used to override the vendor ID used for detecting the NVM >>>> structure allowing to reflash (and authenticate) a new, valid >>>> image on the device. >>>> >>>> Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@klondike.es> >>>> --- >>>> drivers/thunderbolt/switch.c | 9 ++++++++- >>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c >>>> index 3014146081..a7959c3f3f 100644 >>>> --- a/drivers/thunderbolt/switch.c >>>> +++ b/drivers/thunderbolt/switch.c >>>> @@ -13,6 +13,7 @@ >>>> #include <linux/sched/signal.h> >>>> #include <linux/sizes.h> >>>> #include <linux/slab.h> >>>> +#include <linux/moduleparam.h> >>>> #include "tb.h" >>>> @@ -34,6 +35,10 @@ struct nvm_auth_status { >>>> static LIST_HEAD(nvm_auth_status_cache); >>>> static DEFINE_MUTEX(nvm_auth_status_lock); >>>> +static short switch_nvm_vendor_override = -1; >>>> +module_param(switch_nvm_vendor_override, short, 0440); >>>> +MODULE_PARM_DESC(switch_nvm_vendor_override, "Override the switch vendor id on the nvm access routines"); >>>> + >>>> static struct nvm_auth_status *__nvm_get_auth_status(const struct tb_switch *sw) >>>> { >>>> struct nvm_auth_status *st; >>>> @@ -391,7 +396,9 @@ static int tb_switch_nvm_add(struct tb_switch *sw) >>>> * relax this in the future when we learn other NVM formats. >>>> */ >>>> if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL && >>>> - sw->config.vendor_id != 0x8087) { >>>> + sw->config.vendor_id != 0x8087 && >>>> + switch_nvm_vendor_override != PCI_VENDOR_ID_INTEL && >>>> + switch_nvm_vendor_override != 0x8087) { >>>> dev_info(&sw->dev, >>>> "NVM format of vendor %#x is not known, disabling NVM upgrade\n", >>>> sw->config.vendor_id); >>> Patch is corrupted :( >>> >>> Anyway, module parameters are from the 1990's and should stay there. >>> Please use a per-device way to handle this instead, as trying to handle >>> module parameters is very difficult over time. >>> >>> thanks, >>> >>> greg k-h >> Hi Greg! >> >> Thanks for your feedback. I'm a bit uncertain about what you mean with >> a per-device way. Do you mean through the sysfs interface? If so how >> to do so on device discovery time (which is what the Thunderbolt >> driver does)? >> >> I'm surely missing something here so I would really appreciate a >> pointer in the right direction. > Ah, forgot about discovery time, you want this before the device is > probed... > > Then what about the existing "new_id" file for the driver? That's what > it is there for, right? Hi again Greg! "new_id" would work well if the blacklisting was for the whole driver, but currently the driver accepts the corrupted controller just fine but disables the nvm flashing functionality for any vendor IDs it considers inappropriate. The only other approach I can think off is to add a sysfs entry when the vendor missmatches the use can use to try force enabling flashing for a specific vendor ID and have that create the nvm entries if not already there. Do you think this would be better? Thanks for your guidance! Francisco [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 128291 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] thunderbolt: allow vendor ID override for NVM programming 2021-11-24 18:56 ` klondike @ 2021-11-25 6:02 ` Greg Kroah-Hartman 0 siblings, 0 replies; 9+ messages in thread From: Greg Kroah-Hartman @ 2021-11-25 6:02 UTC (permalink / raw) To: klondike Cc: linux-usb, Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Lukas Wunner, Jonathan Corbet, linux-doc On Wed, Nov 24, 2021 at 07:56:50PM +0100, klondike wrote: > El 24/11/21 a las 19:37, Greg Kroah-Hartman escribió: > > On Wed, Nov 24, 2021 at 07:32:29PM +0100, klondike wrote: > >> El 24/11/21 a las 19:26, Greg Kroah-Hartman escribió: > >>> On Wed, Nov 24, 2021 at 05:37:05PM +0100, Francisco Blas Izquierdo Riera (klondike) wrote: > >>>> Currently, the vendor ID reported by the chipset is checked before to > >>>> avoid accidentally programming devices from unsupported vendors with > >>>> a different NVM structure. > >>>> > >>>> Certain Thunderbolt devices store the vendor ID in the NVM, therefore > >>>> if the NVM has become corrrupted the device will report an invalid > >>>> vendor ID and reflashing will be impossible on GNU/Linux even if the > >>>> device can boot in safe mode. > >>>> > >>>> This patch adds a new parameter ``switch_nvm_vendor_override`` which > >>>> can be used to override the vendor ID used for detecting the NVM > >>>> structure allowing to reflash (and authenticate) a new, valid > >>>> image on the device. > >>>> > >>>> Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@klondike.es> > >>>> --- > >>>> drivers/thunderbolt/switch.c | 9 ++++++++- > >>>> 1 file changed, 8 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > >>>> index 3014146081..a7959c3f3f 100644 > >>>> --- a/drivers/thunderbolt/switch.c > >>>> +++ b/drivers/thunderbolt/switch.c > >>>> @@ -13,6 +13,7 @@ > >>>> #include <linux/sched/signal.h> > >>>> #include <linux/sizes.h> > >>>> #include <linux/slab.h> > >>>> +#include <linux/moduleparam.h> > >>>> #include "tb.h" > >>>> @@ -34,6 +35,10 @@ struct nvm_auth_status { > >>>> static LIST_HEAD(nvm_auth_status_cache); > >>>> static DEFINE_MUTEX(nvm_auth_status_lock); > >>>> +static short switch_nvm_vendor_override = -1; > >>>> +module_param(switch_nvm_vendor_override, short, 0440); > >>>> +MODULE_PARM_DESC(switch_nvm_vendor_override, "Override the switch vendor id on the nvm access routines"); > >>>> + > >>>> static struct nvm_auth_status *__nvm_get_auth_status(const struct tb_switch *sw) > >>>> { > >>>> struct nvm_auth_status *st; > >>>> @@ -391,7 +396,9 @@ static int tb_switch_nvm_add(struct tb_switch *sw) > >>>> * relax this in the future when we learn other NVM formats. > >>>> */ > >>>> if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL && > >>>> - sw->config.vendor_id != 0x8087) { > >>>> + sw->config.vendor_id != 0x8087 && > >>>> + switch_nvm_vendor_override != PCI_VENDOR_ID_INTEL && > >>>> + switch_nvm_vendor_override != 0x8087) { > >>>> dev_info(&sw->dev, > >>>> "NVM format of vendor %#x is not known, disabling NVM upgrade\n", > >>>> sw->config.vendor_id); > >>> Patch is corrupted :( > >>> > >>> Anyway, module parameters are from the 1990's and should stay there. > >>> Please use a per-device way to handle this instead, as trying to handle > >>> module parameters is very difficult over time. > >>> > >>> thanks, > >>> > >>> greg k-h > >> Hi Greg! > >> > >> Thanks for your feedback. I'm a bit uncertain about what you mean with > >> a per-device way. Do you mean through the sysfs interface? If so how > >> to do so on device discovery time (which is what the Thunderbolt > >> driver does)? > >> > >> I'm surely missing something here so I would really appreciate a > >> pointer in the right direction. > > Ah, forgot about discovery time, you want this before the device is > > probed... > > > > Then what about the existing "new_id" file for the driver? That's what > > it is there for, right? > Hi again Greg! > > "new_id" would work well if the blacklisting was for the whole driver, but currently the driver accepts the corrupted controller just fine but disables the nvm flashing functionality for any vendor IDs it considers inappropriate. Then fix the devices to do not have corrupted ids! :) Seriously, what does other operating systems do with these broken devices? > The only other approach I can think off is to add a sysfs entry when > the vendor missmatches the use can use to try force enabling flashing > for a specific vendor ID and have that create the nvm entries if not > already there. Do you think this would be better? I think it would be best to fix the firmware in the devices. What prevents that from happening? thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] thunderbolt: allow vendor ID override for NVM programming 2021-11-24 16:37 ` [PATCH 1/2] thunderbolt: " Francisco Blas Izquierdo Riera (klondike) 2021-11-24 18:26 ` Greg Kroah-Hartman @ 2021-11-25 6:16 ` Mika Westerberg 1 sibling, 0 replies; 9+ messages in thread From: Mika Westerberg @ 2021-11-25 6:16 UTC (permalink / raw) To: Francisco Blas Izquierdo Riera (klondike) Cc: linux-usb, Andreas Noever, Michael Jamet, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Greg Kroah-Hartman, Lukas Wunner, Jonathan Corbet, linux-doc Hi, On Wed, Nov 24, 2021 at 05:37:05PM +0100, Francisco Blas Izquierdo Riera (klondike) wrote: > Currently, the vendor ID reported by the chipset is checked before to > avoid accidentally programming devices from unsupported vendors with > a different NVM structure. > > Certain Thunderbolt devices store the vendor ID in the NVM, therefore > if the NVM has become corrrupted the device will report an invalid > vendor ID and reflashing will be impossible on GNU/Linux even if the > device can boot in safe mode. How this can happen? The NVM upgrade verifies the signature of the new NVM and does not allow upgrade if it does not match. Only way I can see this happens is that the NVM is flashed directly to the flash chip through some external tool like dediprog, or the NVM was corrupted before it was signed at Intel which should not happen either (but OK, mistakes can happen). Can you give some more details about the issue? Which device it is and how did the NVM ended being invalid? ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] Documentation: explain how to override Thunderbolt Vendor ID 2021-11-24 16:34 [PATCH 0/2] Thunderbolt: allow vendor ID override for NVM programming Francisco Blas Izquierdo Riera (klondike) 2021-11-24 16:37 ` [PATCH 1/2] thunderbolt: " Francisco Blas Izquierdo Riera (klondike) @ 2021-11-24 16:37 ` Francisco Blas Izquierdo Riera (klondike) 1 sibling, 0 replies; 9+ messages in thread From: Francisco Blas Izquierdo Riera (klondike) @ 2021-11-24 16:37 UTC (permalink / raw) To: linux-usb Cc: Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat, Kranthi Kuntala, Rajmohan Mani, Mario.Limonciello, Greg Kroah-Hartman, Lukas Wunner, Jonathan Corbet, linux-doc, Francisco Blas Izquierdo Riera (klondike) Update the Thunderbolt documentation to explain how to use the ``switch_nvm_vendor_override`` parameter to be able to reflash a Thunderbolt controller's NVM reporting an invalid Vendor ID. Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@klondike.es> --- Documentation/admin-guide/thunderbolt.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/admin-guide/thunderbolt.rst b/Documentation/admin-guide/thunderbolt.rst index 2ed79f41a4..15c8c73331 100644 --- a/Documentation/admin-guide/thunderbolt.rst +++ b/Documentation/admin-guide/thunderbolt.rst @@ -296,6 +296,16 @@ information is missing. To recover from this mode, one needs to flash a valid NVM image to the host controller in the same way it is done in the previous chapter. +When the NVM image is corrupted, certain host controllers will report +an invalid Vendor ID preventing the module from exposing the nvm access +nodes. If so, either load the thunderbolt module with option +``switch_nvm_vendor_override=vendorID`` or add +``thunderbolt.switch_nvm_vendor_override=vendorID`` to your boot +parameters. You need to set ``vendorID`` to the appropriate value. +On Intel systems, this is likely ``0x8086`` but you should verify this +before doing anything as otherwise you may cause further damage to your +controller. + Networking over Thunderbolt cable --------------------------------- Thunderbolt technology allows software communication between two hosts ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-11-25 6:19 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-11-24 16:34 [PATCH 0/2] Thunderbolt: allow vendor ID override for NVM programming Francisco Blas Izquierdo Riera (klondike) 2021-11-24 16:37 ` [PATCH 1/2] thunderbolt: " Francisco Blas Izquierdo Riera (klondike) 2021-11-24 18:26 ` Greg Kroah-Hartman 2021-11-24 18:32 ` klondike 2021-11-24 18:37 ` Greg Kroah-Hartman 2021-11-24 18:56 ` klondike 2021-11-25 6:02 ` Greg Kroah-Hartman 2021-11-25 6:16 ` Mika Westerberg 2021-11-24 16:37 ` [PATCH 2/2] Documentation: explain how to override Thunderbolt Vendor ID Francisco Blas Izquierdo Riera (klondike)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox