[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 08/14] vpci/header: program p2m with guest BAR view
- To: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 2 Feb 2022 11:34:55 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; 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=95PsvAmeP66izDrqOuCBE2Q9rk6iqZqhlUDjN1UpJ/I=; b=kxofX+uLlwRSG0c8Gpfr/k0NVH9UG9vTY0STotr+KJ1YIg2CRTaeZj3SlMZcasx1VDgsfeHaTdMXHiqt14Ft/sBXWgXonRBwMqTHx76PQlqL9lHX/H3u9Re4J5oxdPTtSPIS1/cscc+fHZHtQVRuQHab1wbB7p9yUP07wslK2SgIUTKQlyV8wK9lN1BDR2fthtn36A1Fz4SF6nrFUf8mFsw/auQ3KRMq2f/KU0fjD3Inqo3Wh8updrhYn083WSBXeL6Iv67JxEuOwPZTKcxNQgRQq/g5PkgXCdH0gq1GRqurrApYZp46kTdOaqIJg86TSQXeBaGlDXUfvo2DVrnrrw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Lw5N+buo2mgJVfZM9GR8Ts+UuGK9Yn6N5nqmdJE1HOlAivZJ8n+uWEYVBYVaQ4LqYENPD9s/pAnyv2AoQHm8xf4N+s/O5pCUGwtfyQjOV62fUWFFecFQLHEthwUOh2cxQC7MTHpfe9PbVqGszOZHOxP2b8IvqxwIcTywPnymx/TG6oji2l/CvHcuBwMxF7hDghczvOeV+mfuxcywyzUDHZ3QgnVLa5eBc0zUNhlMW6AH+RQ31Uqkna5obwmkv27aaa3PCCWAJbyPv1F8vJa45Uq4M4WeA7HvTBLIIjfhdoIpyp7zrc0e7N1irV252Ng/22l8iyVTBK36WIBsVeTVcA==
- Authentication-results: esa6.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
- Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>, "jbeulich@xxxxxxxx" <jbeulich@xxxxxxxx>, "andrew.cooper3@xxxxxxxxxx" <andrew.cooper3@xxxxxxxxxx>, "george.dunlap@xxxxxxxxxx" <george.dunlap@xxxxxxxxxx>, "paul@xxxxxxx" <paul@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>
- Delivery-date: Wed, 02 Feb 2022 10:35:25 +0000
- Ironport-data: A9a23:l+blKa4MpKfqsnX4MsILowxRtBjBchMFZxGqfqrLsTDasY5as4F+v mJLUDjVMv3ZY2ujcot0OY+ypEtXuJXcxtMyQQZo+HthHi5G8cbLO4+Ufxz6V8+wwmwvb67FA +E2MISowBUcFyeEzvuV3zyIQUBUjclkfJKlYAL/En03FV8MpBsJ00o5wbZj2tQw2LBVPivW0 T/Mi5yHULOa82Yc3lI8s8pvfzs24ZweEBtB1rAPTagjUG32zhH5P7pGTU2FFFPqQ5E8IwKPb 72rIIdVXI/u10xF5tuNyt4Xe6CRK1LYFVDmZnF+A8BOjvXez8CbP2lS2Pc0MC9qZzu1c99Zl 8cQhMG0Vy4QM7SWnqdHXyhfQwR0BPgTkFPHCSDXXc27ykTHdz3nwul0DVFwNoodkgp1KTgQr 7pCcmlLN03dwbLtqF64YrAEasALNs7kMZlZonh95TrYEewnUdbIRKCiCdpwgmxq3JoWQam2i 8wxNSBgbAWbUix0eVIKS8gUnfbzj1n4WmgNwL6SjfVuuDWCpOBr65DvOtfIft2BRe1Og12V4 GnB+gzRKwsGOdmo7CuK+3OhmMfChSr+HokVEdWQ7vd3hHWDy2pVDwcZPXOhqPmkjgilWtRQK 2Qd4C9opq83nGShQ8PhRRS+rDiBtwQFRttLO+Qg7UeGza+8yzieAm8IXztQcusMvcU9RSEp/ lKRltavDjtq2JWFRHTY+rqKoDeaPSkOMXREdSICVREC4dTovMc0lB2nZvFnHa2uh9v5AwbZx TyQsTM+jLUei80M/6ij9FWBiDWpzrDLUwo06wP/Tm+jqARja+aNQIil6kPS6/paG7qIVVmKv HUCmM+24fgHCNeGkynlaP4WALij6vKBMTvdqV1iBZ8s83Kq4XHLQGxLyGggfgEzaJ9CIGK3J h+I0e9M2HNNFCS4MJ4qZ5yYMskzl66jT9jUUaDxZOMbN/CdazS71C1pYEeR2UXkn04tjbwzN P+nTCq8MZoJIf85lWTrHo/xxZdun3ljnj2LGfgX2jz6ieL2WZKDdVsS3LJihMgd5bjMngja+ s032yCim0QGC72WjsU6HOcuwbE2wZoTWcGeRy9/LLfrzu9a9IYJUKe5/F/ZU9Y595m5b8+Rl p1HZmdWyUDkmVrMIhiQZ3ZoZdvHBMgj9i1nZH19YA/2ixDPhLpDC49EL/MKkUQPrrQ/nZaYs dFZEyl/Phi/YmueoGlMBXUMhIdjaA6qlWqz09mNO1ACk2pbb1WRoLfMJ1K3nAFXV3bfnZZg/ 9WIi12KKbJeF1UKJJuHM5qHkgLu1UXxbcovBSMk1PEJJhW1mGWrQgSs5sIKzzYkckSen2bKi lrNXH/1Z4Dl+ucIzTUAvojdx6+BGOpiBEtKWW7d6Le9Ly7B+WS/h4RHVY61kfr1DgsYIY2uO rdYye/SKvoCkAoYuoZwCe8zn6k/+8Hut/lRyQE9RCfHaFGiC7VBJHia3JYQ6v0Rl+EB4QbmC FiS/tR6OKmSPJ+3GlAmOwd4PP+I0usZm2eO4K1tcln6/iJ+4JGOTV5WY0uXkCVYIbYsaNElz O4ttdQ48Qu6jhZ2YN+KgjoNrzaHL2AaUrVhvZYfWde5hg0uw1BEQJrdFi6pv83fN4QSahEne 2bGirDDirJQwlv5X0AyTXWdj/BAgZkuuQxRyANQLVq+hdeY1OQ82wdc8GprQ10NnAlHye96J kNiK1ZxefeV5z5ticVOAzKsFgVGCEHL80D90QJUxmjQTk3uXW3RNmwtf+2K+RlBoW5bezFa+ pCeyXrkDmm2LJ2ggHNqVB43seHnQPxw6hbGyZKuEMmyFpUnZSbo3/21bm0Sphq7Wc48iSUrf wWxEDqcvUEjCRMtng==
- Ironport-hdrordr: A9a23:WX4+X6z6WE6jNfe72py+KrPxtOskLtp133Aq2lEZdPULSKOlfp GV8MjziyWYtN9wYhAdcdDpAtjmfZr5z+8O3WB3B8beYOCGghrSEGgG1+XfKlLbak/DH4JmpM Jdmu1FeaHN5DtB/LfHCWuDYq8dKbC8mcjC74eurEuFDzsaE52Ihz0JdDpzeXcGIjWua6BJcK Z1saF81kWdkDksH4+GL0hAe9KGi8zAlZrgbxJDLxk76DOWhTftzLLhCRCX0joXTjsKmN4ZgC X4uj28wp/mn+Cwyxfa2WOWx5NKmOH5wt8GIMCXkMAaJhjllw7tToV8XL+puiwzvYiUmRwXue iJhy1lE9V46nvXcG3wiRzx2zP42DJr0HPmwU/wuwqUneXJABYBT+ZRj4NQdRXUr2A6ustn7a 5N12WF87JKEBLphk3Glpb1fiAvsnDxjWspkOYVgXAae5AZcqVtoYsW+14QOIscHRj99JssHI BVfY7hDc5tABOnhk3izypSKITGZAVwIv7GeDlPhiWt6UkWoJgjpHFogfD2nR87heUAotd/lq D5259T5cJzp/ktHNZA7dc6MLuK41P2MGDx2UKpUB3a/fI8SjrwQ6Ce2sRB2AjtQu1O8KcP
- Ironport-sdr: HOhXjLI1DNdmgZ5v6J5Iv9CX1v160vwBuYyfuDaLqEe2UkpaDf2zsRJv3n7k5VuL6UTbUNWmPC 7IE4HnPZgoSx/jy2P1ktgYGBo7D7ePgzoq84LkdiYbnV7CJmXupvlF1HErWvjuX+nVQNA+bneB Kn6y5LMQUvTHxCn7c7PtZ6rv9a6OG7lfgOANVQHQfgovFTRvkF/QEKDyxLgnU0U4IRRasQfsCv aAuzvsK38+05h/uFZY4lT2xBmwDhdWsRXKU2je4T8UDtN9Th30yq2yPlntEOl7Vr88NsNAhmxz V/flmrfKbHgggeMD2its9+J1
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Wed, Feb 02, 2022 at 09:46:21AM +0000, Oleksandr Andrushchenko wrote:
>
> >>> + gdprintk(XENLOG_G_DEBUG,
> >>> + "%smap [%lx, %lx] -> %#"PRI_gfn" for %pd\n",
> >>> + map->map ? "" : "un", s, e, gfn_x(start_gfn),
> >>> + map->d);
> >> That's too chatty IMO, I could be fine with printing something along
> >> this lines from modify_bars, but not here because that function can be
> >> preempted and called multiple times.
> > Ok, will move to modify_bars as these prints are really helpful for debug
> I tried to implement the same, but now in init_bars:
>
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 667c04cee3ae..92407e617609 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -57,10 +57,6 @@ static int map_range(unsigned long s, unsigned long e,
> void *data,
> */
> start_gfn = gfn_add(start_gfn, s - mfn_x(start_mfn));
>
> - gdprintk(XENLOG_G_DEBUG,
> - "%smap [%lx, %lx] -> %#"PRI_gfn" for %pd\n",
> - map->map ? "" : "un", s, e, gfn_x(start_gfn),
> - map->d);
> /*
> * ARM TODOs:
> * - On ARM whether the memory is prefetchable or not should be
> passed
> @@ -258,6 +254,28 @@ static void defer_map(struct domain *d, struct pci_dev
> *pdev,
> raise_softirq(SCHEDULE_SOFTIRQ);
> }
>
> +static int print_range(unsigned long s, unsigned long e, void *data)
> +{
> + const struct map_data *map = data;
> +
> + for ( ; ; )
> + {
> + gfn_t start_gfn = _gfn(PFN_DOWN(is_hardware_domain(map->d)
> + ? map->bar->addr
> + : map->bar->guest_reg));
> + mfn_t start_mfn = _mfn(PFN_DOWN(map->bar->addr));
> +
> + start_gfn = gfn_add(start_gfn, s - mfn_x(start_mfn));
> +
> + gdprintk(XENLOG_G_DEBUG,
> + "%smap [%lx, %lx] -> %#"PRI_gfn" for %pd\n",
> + map->map ? "" : "un", s, e, gfn_x(start_gfn),
> + map->d);
> + }
This is an infinite loop AFAICT. Why do you need the for for?
> +
> + return 0;
> +}
> +
> static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool
> rom_only)
> {
> struct vpci_header *header = &pdev->vpci->header;
> @@ -423,7 +441,25 @@ static int modify_bars(const struct pci_dev *pdev,
> uint16_t cmd, bool rom_only)
> if ( !map_pending )
> pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
> else
> + {
> + struct map_data data = {
> + .d = pdev->domain,
> + .map = cmd & PCI_COMMAND_MEMORY,
> + };
> +
> + for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
> + {
> + const struct vpci_bar *bar = &header->bars[i];
> +
> + if ( rangeset_is_empty(bar->mem) )
> + continue;
> +
> + data.bar = bar;
> + rc = rangeset_report_ranges(bar->mem, 0, ~0ul, print_range,
> &data);
Since this is per-BAR we should also print that information and the
SBDF of the device, ie:
%pd SBDF: (ROM)BAR%u %map [%lx, %lx] -> ...
> + }
> +
> defer_map(dev->domain, dev, cmd, rom_only);
> + }
>
> return 0;
>
>
> To me, to implement a single DEBUG print, it is a bit an overkill.
> I do understand your concerns that "that function can be
> preempted and called multiple times", but taking look at the code
> above I think we can accept that for DEBUG builds.
It might be better if you print the per BAR positions at the top of
modify_bars, where each BAR is added to the rangeset? Or do you care
about reporting the holes also?
Thanks, Roger.
|