[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] PCI: don't allow "pci-phantom=" to mark real devices as phantom functions


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Thu, 5 May 2022 19:10:20 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=KGA2hZsAl529ZEtCEJAY/LM2QSUI5YvIhNHixGn8xfg=; b=H3ebwcTRmEyaaBh4jE5h/EgDW1RHK7KE5jQ1lSX9LImRtsmYtcg75fLsNH0AaKNPWEdaCHxwEIElTtzsjnTXNY8WkXQelMXFZDGwlebhUUq6g8i1PVnDGb0u9G4IoTtobpexsaBLvlMlcHBfqlZ5VSc9H39HdnHThntWeKnhgbVR47a0UNFUaWWqTMzKbIQzikDMY9I6VgsRs02gtLMkoIow5ZjB2pEFWGW2ndvgWj63MdmAMtvFo7WLCK+rqfxuZVANCzgmVS95Kat+gv+cPQIhhCi3Wnm5gu3xE9tpLLFfnMNGA7T/DY/ylATE64rZ70c6iAQnMeUHJ6Hq+/7cVA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gMDv9m0LP2WsWR7QxHckYU/WRknZmswGk1LYqoRCSQ1gp6LNrReq4DF/rz5+w5DW0a2vSTuLBgeK15b3gk+OEN0i4G14S7S8FNYsbb7X1enTrqy2loPM4BuClhEI7wzdY35MegGzPQJmeTNx98iZJ8ZFzLlUTI2Y49YUT5hdIaBiVJaq1WZb72XnGlPhGRQiS80TXtMRtVNBvg92ex2Sl5wEnKnQq8E6e/5EPRwjY5fzEWu/98AbDY3ZMhboNtIx7lbR7m49SFagVh9D5cgPlQ8SIf7gvrVPh7KnU1Qj9KJMSAC/Q5I+fs1U5ueXYlgnS1sQBw5g0Eml0B6PqlCAog==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Thu, 05 May 2022 19:10:37 +0000
  • Ironport-data: A9a23:MOeEd6Pf7bqsW9fvrR3YlsFynXyQoLVcMsEvi/4bfWQNrUp30GRWz GAZXjiOO/aKYGf3coh3Otiy/UlSu8fXzNJjTQto+SlhQUwRpJueD7x1DKtR0wB+jCHnZBg6h ynLQoCYdKjYdleF+lH1dOKJQUBUjclkfJKlYAL/En03FFYMpBsJ00o5wbZk2tMw3bBVPivW0 T/Mi5yHULOa82Yc3lI8s8pvfzs24ZweEBtB1rAPTagjUG32zhH5P7pGTU2FFFPqQ5E8IwKPb 72rIIdVXI/u10xF5tuNyt4Xe6CRK1LYFVDmZnF+A8BOjvXez8CbP2lS2Pc0MC9qZzu1c99Z7 dB0hKK7EV4TJKDoge0UAzl6NRxlMvgTkFPHCSDXXc276WTjKiGp79AwSUY8MMsf5/p9BnxI+ boAMjcRYxufhuWwhrWmVu1rgcdlJ87uVG8dkig4kXeFUrB7ENaaHfWiCdxwhV/cguhnG/rEa tVfQj1odBnaODVEO0sNCYJ4l+Ct7pX6W2IA8A3I+/ppi4TV5FxM7OLIL8rQQ/mhX+NeuV63i EjD812sV3n2M/Tak1Jp6EmEhOXCgCf6U4I6D6Cj+7hhh1j77nweDlgaWEW2pdG9i1WiQJRPJ koM4C0soKMuskuxQbHVRxSlpFaUsxhaXMBfe9DW8ymIw6vQpgqcWG4NS2cbbMR87ZFmAzs3y lWOgtXlQyR1t6GYQm6c8bHSqi6uPS8SLikJYipsoRY53uQPabob1nrnJuuP2obu5jEpMVkcG wy3kRU=
  • Ironport-hdrordr: A9a23:Qv+xhaFsyqAfhOd9pLqFsZLXdLJyesId70hD6qkvc3Fom52j/f xGws5x6fatskdrZJkh8erwW5Vp2RvnhNJICPoqTM2ftW7dySSVxeBZnMbfKljbdxEWmdQtsp uIH5IeNDS0NykDsS+Y2nj3Lz9D+qjgzEnAv463oBlQpENRGthdBmxCe2Sm+zhNNW177O0CZf +hD6R8xwaISDAyVICWF3MFV+/Mq5ngj5T9eyMLABYh9U2nkS6owKSSKWnZ4j4uFxd0hZsy+2 nMlAL0oo+5teug9xPa32jPq7xLhdrazMdZDsDksLlXFtyssHfrWG1SYczHgNkHmpDp1L/sqq iLn/4UBbU315oWRBDtnfKi4Xi57N9k0Q6e9bbRuwqenSW+fkN6NyMJv/MmTvOSgXBQw+1Uwe ZF2XmUuIFQCg6FlCPh58LQXxUvjUasp2E++NRjx0C3fLFuHoO5l7ZvtX+90a1wbh7S+cQiCq 1jHcvc7PFZfReTaG3YpHBmxJipUm4oFhmLT0AesojNugIm1kxR3g8d3ogSj30A/JUyR91N4P nFKL1hkPVLQtUNZaxwCe8dSY+8C3DLQxjLLGWOSG6XX50vKjbIsdr68b817OaldNgBy4Yzgo 3IVBdCuWs7ayvVeLqzNV1wg2TwqUmGLETQI5tllulEU5XHNcnWGDzGTkwymM29pPhaCtHHWp +ISedrP8M=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYW8nVI0Vwkw76REOxFvedZ2w8H60Qr/GA
  • Thread-topic: [PATCH] PCI: don't allow "pci-phantom=" to mark real devices as phantom functions

On 29/04/2022 14:05, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
> unless you have verified the sender and know the content is safe.
>
> IOMMU code mapping / unmapping devices and interrupts will misbehave if
> a wrong command line option declared a function "phantom" when there's a
> real device at that position. Warn about this and adjust the specified
> stride (in the worst case ignoring the option altogether).
>
> Requested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -451,7 +451,24 @@ static struct pci_dev *alloc_pdev(struct
>                           phantom_devs[i].slot == PCI_SLOT(devfn) &&
>                           phantom_devs[i].stride > PCI_FUNC(devfn) )
>                      {
> -                        pdev->phantom_stride = phantom_devs[i].stride;
> +                        pci_sbdf_t sbdf = pdev->sbdf;
> +                        unsigned int stride = phantom_devs[i].stride;
> +
> +                        while ( (sbdf.fn += stride) > PCI_FUNC(devfn) )

I'm fairly sure this doesn't do what you want it to.

.fn is a 3 bit bitfield, meaning the += will be truncated before the
compare.

> +                        {
> +                            if ( pci_conf_read16(sbdf, PCI_VENDOR_ID) == 
> 0xffff &&
> +                                 pci_conf_read16(sbdf, PCI_DEVICE_ID) == 
> 0xffff )
> +                                continue;
> +                            stride <<= 1;
> +                            printk(XENLOG_WARNING
> +                                   "%pp looks to be a real device; bumping 
> %04x:%02x:%02x stride to %u\n",
> +                                   &sbdf, phantom_devs[i].seg,
> +                                   phantom_devs[i].bus, phantom_devs[i].slot,
> +                                   stride);
> +                            sbdf = pdev->sbdf;
> +                        }
> +                        if ( PCI_FUNC(stride) )

This is an obfuscated way of writing stride < 8.

Given the printk(), if we actually find an 8-function device, what gets
printed (AFAICT) will be "bumping to 8" when in fact we mean "totally
ignoring the option".  I think this really wants an else clause.

~Andrew

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.