RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4

[Mario.Limonciello]

> -----Original Message-----
> From: Limonciello, Mario
> Sent: Tuesday, October 1, 2019 12:05 PM
> To: 'Mika Westerberg'; linux-usb@xxxxxxxxxxxxxxx
> Cc: Andreas Noever; Michael Jamet; Yehezkel Bernat; Rajmohan Mani; Nicholas
> Johnson; Lukas Wunner; Greg Kroah-Hartman; Alan Stern; Anthony Wong; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
> 
> > @@ -322,9 +398,21 @@ static int tb_switch_nvm_add(struct tb_switch *sw)
> >  	u32 val;
> >  	int ret;
> >
> > -	if (!sw->dma_port)
> > +	if (!nvm_readable(sw))
> >  		return 0;
> >
> > +	/*
> > +	 * The NVM format of non-Intel hardware is not known so
> > +	 * currently restrict NVM upgrade for Intel hardware. We may
> > +	 * relax this in the future when we learn other NVM formats.
> > +	 */
> > +	if (sw->config.vendor_id != PCI_VENDOR_ID_INTEL) {
> > +		dev_info(&sw->dev,
> > +			 "NVM format of vendor %#x is not known, disabling NVM
> > upgrade\n",
> > +			 sw->config.vendor_id);
> > +		return 0;
> > +	}
> > +
> 
> Don't you actually have an attribute you can use here for this exact purpose that
> you
> could  be setting rather than returning immediately?
> sw->no_nvm_upgrade
> 

One more thought; would you consider exporting to sysfs sw->config.vendor_id?
Maybe an attribute that is switch_vendor?

Userland fwupd also does validation on the NVM and will need to follow this.
The same check will go into fwupd to match the vendor and lack of nvm_non_active0
to mark the device as not updatable.  When the checks in the kernel get relaxed,
some NVM parsing will have to make it over to fwupd too to relax the check at that point.

> Then potentially you can at least let users "dump out" the nvm on !Intel but don't
> let
> it be written until ready to relax further.
> 
> >  	nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
> >  	if (!nvm)
> >  		return -ENOMEM;