[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] xen/arm: pci: fix check in pci_check_bar()
- To: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 12 Jul 2023 09:12:36 +0200
- 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=iZDuUCeOei3et9f1Pwy3Ob68Ck5ILt6WMR/7aoeuyHo=; b=YpNvcyPVTwikOoRZIBe24xpZHfmKC1tcV5Us6koetV2wuLG4vvImVZxjrQdCYNwut/H5F3cfn4xXSt7yaYk9gD/An/jYfw6zH27eAIC+dy7Fv5guNiyJT+f9t51JYwjnlld+RILq3fhfOGr0vslT7WDY1wHg5w8p3hX7rMEkSegzBMkjV0PUb22wuJoLgvaw0IsF3wyMDDpDXhH0rln4sYGkLqV3G83ePqf8oBY+5r0UD84Vxc5gaNujvWp/9K4gKRs+uTjzR93Kd9c7Rhv5NOktURuInE3dZrXwPefE909XlZmDYLtV04aYb5SYu+Xe3yy9jUeQuM2aNtwMpi57eA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FsLsnH6LWQajf5iCXiSDyZvxfrQGNAGBZ3N02ejzqGcbqKkwmuW5qiYUVyRw2fPWrnkRv/PLffdXVt6JrxOJvhVHUH3N7jruYmELb28E2CulcbfeMhm1Eea2B+N7pgVVkjQrLaWvkTdr8w4YbBrzt0kat2vuFRZPEx10fIhJJiueeCyxNBPfyM8sFSYqrb3aQ34qFzH90xcCQtUNyazhY99UQegd8F/uy7CiA+yNKj4DB8gITckYT2/RdDcQjGW93KpEvP8MNLwZWs8zd4nMSswnDxwrevlClOp5YDqYCJY1XDXorhrQF3o2IMV+bM9lwPAvQRyHrAo0VyxgpWdG/Q==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>
- Delivery-date: Wed, 12 Jul 2023 07:13:07 +0000
- Ironport-data: A9a23:gOaWLKr7Cr4n2BYd6t8WEIiBLWJeBmKPZBIvgKrLsJaIsI4StFCzt garIBnTb6mJMDD9et50bIy0pEtU7JTcytRkQAA4qiswECIS9puZCYyVIHmrMnLJJKUvbq7FA +Y2MYCccZ9uHhcwgj/3b9ANeFEljfngqoLUUbKCYWYpA1c/Ek/NsDo788YhmIlknNOlNA2Ev NL2sqX3NUSsnjV5KQr40YrawP9UlKq04GpwUmAWP6gR5weBziNNVvrzGInqR5fGatgMdgKFb 76rIIGRpgvx4xorA9W5pbf3GmVirmn6ZFXmZtJ+AsBOszAazsAA+v9T2Mk0MC+7vw6hjdFpo OihgLTrIesf0g8gr8xGO/VQO3kW0aSrY9YrK1Dn2SCY5xWun3cBX5yCpaz5VGEV0r8fPI1Ay RAXAG9OTDK/ltKb+uKicbBXm9sxIZXpf6pK7xmMzRmBZRonabbqZvyQoPpnhnI3jM0IGuvCb c0EbzYpdA7HfxBEJlYQDtQ5gfusgX78NTZfrTp5p4JuuzSVkFM3juarbIK9lt+iHK25mm6Co W3L5SLhCwwyP92D0zuVtHmrg4cjmAuiAdJPTOPprqICbFu76FNKCUxGEn6Ap8LlzU+dd+9kc g9I0397xUQ13AnxJjXnZDWjvHObtwQAHdpRF+E34huEzKb86gOVQGMDS1ZpeNEg8cM7WzEu/ luIhM/yQyxitqWPTnCQ/avSqim9UQAONnMLbyIASQoD4vHgrZs1gxaJScxseIa3k9n0FDfY0 z2M6i8kiN07h8MRy7+y+1yBhju2v4XIVSY8/ACRVWWghitHY4qia52t+ELsx/9KJ4aETXGMp HEB3cOZ6YgmF5iNiSjLW+QLE7GB7uyAdjbbhDZS84IJ8j2s/zumYtpW6TQnfkNxaJ5YIXnuf VPZvh5X6NlLJny2YKRrYoW3TcM30aznEtejXffRBjZTXqVMmMa81HkGTSatM6rFySDATYlX1 U+nTPuR
- Ironport-hdrordr: A9a23:YyWOd62ZCUdlklEVNsS/MgqjBU1yeYIsimQD101hICG9Lfb3qy n+ppsmPEHP5Ar5AEtQ6uxpOMG7MAjhHO1OkPss1NaZLX/bUQ6TRuBfBOTZskLd8kHFh5dgPO JbAthD4b7LfBFHZKTBkXeF+r8bqbHtncDY5pa7vhAdKz2Gc5sQijuRSDzrY3GeLzM2f6bRYa Dsmvav0ADQBEj/AP7LfkXta9Kz7+Ej2aiWISIuNloC0k2jnDmo4Ln1H1yx2QofaSpGxfMY/W 3Mg2XCl+qeW6XQ8G6/60bjq7Bt3PfxwNpKA8KBzuIPLC/3twqubIN9H5WfoTEcuoiUmQ8Xue iJhy1lE9V46nvXcG3wiwDqwRPc3DEn7GKn4UOEgEHkvdfySFsBeox8bLpiA0PkAncbzZVBOe NwriekXqNsfFH9dfHGlp/1vxIDrDv5nZNtq59Js5UVa/pqVFZrl/1TwKpkKuZKIMuz0vFSLA BQNrCX2B93SyLVU5mLhBgv/DXrZAVxIj62BnEYvMqbyj5Xm2084Xc56aUk7yo93aN4coJD4e vcNKRuifVpde85KYxAJMppe7rsNoXie2O6DF6v
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Tue, Jul 11, 2023 at 03:28:28PM -0400, Stewart Hildebrand wrote:
> When mapping BARs for vPCI, it's valid for a BAR start address to equal the
> BAR
> end address (i.e. s == e) since e is inclusive. However, pci_check_bar()
> currently returns false in this case, which results in Xen not mapping the
> BAR.
> In this example boot log, Linux has mapped the BARs, but since Xen did not map
> them, Linux encounters a data abort and panics:
I would maybe reword this a bit to clarify that Xen has not mapped the
BARs in the guest second stage page tables, 'Xen is not mapping the
BAR' is IMO too vague.
>
> [ 2.593300] pci 0000:00:00.0: BAR 0: assigned [mem 0x50008000-0x50008fff]
> [ 2.593682] pci 0000:00:00.0: BAR 2: assigned [mem 0x50009000-0x50009fff]
> [ 2.594066] pci 0000:00:00.0: BAR 4: assigned [mem 0x5000a000-0x5000afff]
> ...
> [ 2.810502] virtio-pci 0000:00:00.0: enabling device (0000 -> 0002)
> (XEN) 0000:00:00.0: not mapping BAR [50008, 50008] invalid position
> (XEN) 0000:00:00.0: not mapping BAR [50009, 50009] invalid position
> (XEN) 0000:00:00.0: not mapping BAR [5000a, 5000a] invalid position
> [ 2.817502] virtio-pci 0000:00:00.0: virtio_pci: leaving for legacy driver
> [ 2.817853] virtio-pci 0000:00:00.0: enabling bus mastering
> (XEN) arch/arm/traps.c:1992:d0v0 HSR=0x00000093010045 pc=0xffff8000089507d4
> gva=0xffff80000c46d012 gpa=0x00000050008012
> [ 2.818397] Unable to handle kernel ttbr address size fault at virtual
> address ffff80000c46d012
> ...
>
> Since e is inclusive, drop the equality check.
>
> Also, adjust e to include the whole page. This increases the accuracy of the
> subsequent is_bar_valid check.
I think you want to reorder those sentences, when e is adjusted to
account for the full page s == e is actually impossible, hence the =
part of the check can be dropped:
"Adjust the end physical address to account for the full page when
converting from mfn, at which point start and end cannot be equal, so
drop the equal check in the condition."
Or something similar.
The rest LGTM.
>
> Fixes: cc80e2bab0d0 ("xen/pci: replace call to is_memory_hole to
> pci_check_bar")
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
With the above adjusted:
Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
Thanks, Roger.
|
