Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   <Mario.Limonciello@dell.com>
To:     <mika.westerberg@linux.intel.com>, <linux-usb@vger.kernel.org>
CC:     <andreas.noever@gmail.com>, <michael.jamet@intel.com>,
        <YehezkelShB@gmail.com>, <rajmohan.mani@intel.com>,
        <nicholas.johnson-opensource@outlook.com.au>, <lukas@wunner.de>,
        <gregkh@linuxfoundation.org>, <stern@rowland.harvard.edu>,
        <anthony.wong@canonical.com>, <linux-kernel@vger.kernel.org>
Subject: RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
Thread-Topic: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
Thread-Index: AQHVeEzShOhklesInk+gx9St4A7986dGAqiggAATYiA=
Date:   Tue, 1 Oct 2019 18:14:23 +0000
Message-ID: <5357cb96013445d79f5c2016df8a194e@AUSX13MPC105.AMER.DELL.COM>
References: <20191001113830.13028-1-mika.westerberg@linux.intel.com>
 <20191001113830.13028-18-mika.westerberg@linux.intel.com>
 <184c95fc476146939b240557e54ee2c9@AUSX13MPC105.AMER.DELL.COM>
In-Reply-To: <184c95fc476146939b240557e54ee2c9@AUSX13MPC105.AMER.DELL.COM>
Accept-Language: en-US
Content-Language: en-US
msip_labels: MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Enabled=True;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_SiteId=945c199a-83a2-4e80-9f8c-5a91be5752dd;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Owner=Mario_Limonciello@Dell.com;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_SetDate=2019-10-01T17:05:07.4414478Z;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Name=External Public;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Application=Microsoft Azure
 Information Protection;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Extended_MSFT_Method=Manual;
 aiplabel=External Public
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.143.18.86]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,1.0.8
 definitions=2019-10-01_08:2019-10-01,2019-10-01 signatures=0
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0
 mlxlogscore=931 mlxscore=0 impostorscore=0 spamscore=0 lowpriorityscore=0
 priorityscore=1501 bulkscore=0 phishscore=0 clxscore=1015 adultscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-1908290000 definitions=main-1910010147
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=999
 priorityscore=1501 bulkscore=0 mlxscore=0 spamscore=0 lowpriorityscore=0
 suspectscore=0 malwarescore=0 impostorscore=0 adultscore=0 clxscore=1015
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-1908290000
 definitions=main-1910010147
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Limonciello, Mario
> Sent: Tuesday, October 1, 2019 12:05 PM
> To: 'Mika Westerberg'; linux-usb@vger.kernel.org
> Cc: Andreas Noever; Michael Jamet; Yehezkel Bernat; Rajmohan Mani; Nichol=
as
> Johnson; Lukas Wunner; Greg Kroah-Hartman; Alan Stern; Anthony Wong; linu=
x-
> kernel@vger.kernel.org
> Subject: RE: [RFC PATCH 17/22] thunderbolt: Add initial support for USB4
>=20
> > @@ -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 !=3D 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;
> > +	}
> > +
>=20
> Don't you actually have an attribute you can use here for this exact purp=
ose that
> you
> could  be setting rather than returning immediately?
> sw->no_nvm_upgrade
>=20

One more thought; would you consider exporting to sysfs sw->config.vendor_i=
d?
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_a=
ctive0
to mark the device as not updatable.  When the checks in the kernel get rel=
axed,
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.
>=20
> >  	nvm =3D kzalloc(sizeof(*nvm), GFP_KERNEL);
> >  	if (!nvm)
> >  		return -ENOMEM;