[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 <andr2000@xxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 13 Jan 2022 11:22:19 +0100
  • 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=swU79RKhpKmTDx2UQp8knIDGCmys2YDniE22Z0GmeRs=; b=LRVwuyCdBWP2Q3YdsJzuyIDaPy0k4VXOeFM0iSl3NJPF2lIA0r2FsOpf5R55eGRVyNNjzlKRuoG3mTzexRw3C9rEJbEuD7/LFnWfQ9s+/hBvSuntmPdvqrsfiYNHgZzUg9Hey6XOpsP8QYdGloq0UMIS2HyNFFcXV0nnEgBynu8SITvlBqEOZuilmacllXmzrz5QCvYoG94W7oqpg6d2EQdwpP1I1OOW6Owad9fWCDEz0cabCnY+6ZUmtf1NDfuDT0TwQxVEIQlCVmfAoqJFHFfyCsHKM5Rey8S/lVUGTdFJt8XkXB8jYoTxxFqLRYgFEqTyBLdpU1Y/4gwhmMynBA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gVJGJHH71dJZntvuPRAyCm4YLeJ5LI8hXLEyknB2WuO6W1G1iHErQz93/o88+ao9epc6HlakPQP0ZIembun5rRSU7VGZ2bR7RIVR6DD6uWg12NWzeg35wInzNohPJYCT+7WH09rqyFVEM8n6EjEBjWKC+ZWO8t0pAn4YzSmWfs2YBGToPL1b0J+NmnPfWtlTN1o/2RWCW9Xf1yKAdKoNjMPnUzOd+l+TXb670GhZvFqKOZjrzLeEcqMvhSQUirIseLRTiRk6uNAfNF37sNEGNGSUQ3Fiu/cgjTHsiiIyOJnYy9c6x4oyXqyiA45CmACYSDi3cgeNN4yOvI/dQPoyBg==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, <julien@xxxxxxx>, <sstabellini@xxxxxxxxxx>, <oleksandr_tyshchenko@xxxxxxxx>, <volodymyr_babchuk@xxxxxxxx>, <Artem_Mygaiev@xxxxxxxx>, <jbeulich@xxxxxxxx>, <andrew.cooper3@xxxxxxxxxx>, <george.dunlap@xxxxxxxxxx>, <paul@xxxxxxx>, <bertrand.marquis@xxxxxxx>, <rahul.singh@xxxxxxx>, Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
  • Delivery-date: Thu, 13 Jan 2022 10:22:40 +0000
  • Ironport-data: A9a23:VjBsS6PJKx9HKlXvrR1lkcFynXyQoLVcMsEvi/4bfWQNrUoh0TJWn GUdDDuBMvzeN2akfd5ybovk/BsCv5DSmtdkSwto+SlhQUwRpJueD7x1DKtR0wB+jCHnZBg6h ynLQoCYdKjYdpJYz/uUGuCJQUNUjMlkfZKhTr6UUsxNbVU8En150Es4w7dRbrNA2rBVPSvc4 bsenOWHULOV82Yc3rU8sv/rRLtH5ZweiRtA1rAMTakjUGz2zhH5OKk3N6CpR0YUd6EPdgKMq 0Qv+5nilo/R109F5tpICd8XeGVSKlLZFVDmZna7x8FOK/WNz8A/+v9TCRYSVatYoz6zx8Jxw /xcicePElsiP4P2o+0iXwYNRkmSPYUekFPGCX22sMjVxEzaaXr8hf5pCSnaP6VBpLwxWzsXs 6VFdnZdNXhvhMrvqF6/YvNrick5atHiIasUu216zCGfBvEjKXzGa/uTtIcIjGps7ixINdrxe ZcmNDpvVTn/eBNuNkkpU4xlvej90xETdBUH8QnI9MLb+VP71AVs1JD9PdyTfcaFLe1XlEuFo mPN/0ziHwoXcteYzFKt22iwi+r4uDL0UYMfCpW17vdvxlaUwwQ7DxkbVkCyp/WjvUe4V8hCM Ewf+icorq8a+VSiS5/2WBjQiGSNvgMYHcFRFeI6wAiXz+zf5APxLmIJVCJbYdoq8so/XyU31 0ShlsnsQzdotdW9S2+Z97qShSO/P24SN2BqTT8JS04J7sfupKk3jwnTVZBzHaitlNr3FDrsh TeQo0AWjrMfl5RTj/2T8lXOgjbqrZ/MJiYr4QHQUnOg/xlOboevbIy16nDW9f9Fao2eSzGpu 3wJmNOX6uwUOo2cjyyGQOgLH7aB6u6MNXvXhlsHN4I66z2n9nqnfIZRyDJzPkFkNoADYzCBX aPIkVoPvtkJZiLsNPIpJdLqYyg38UT+Pd/fDuLUZfNfXsNOL1OM/Q9UXkev03+4xSDAjpoDE ZucdM+tC1MTBqJm0Ce6So8h7FM7+swt7TiNHM6mlnxLxZLbPSfIEuldbDNie8hktPvsnenDz zpI2yJmIT17Wfa2XCTY+JV7wbsifSliXsCeRyC6m4e+zuta9IMJV665LVAJIdUNc0FpegHgp CHVtqhwkguXuJE/AV/WAk2PkZu2NXqFkVo1PDY3IXGj0GU5bICk4c83LsVrJ+F+r7Q9ka4vF ZHpnvls5NwVGlwrHBxHPPHAQHFKLkz31WpiwQL4CNTAQ3KQb1OQoYK1Fuce3CIPEjC2paMDT 06IjWvmrW44b106Vq7+Mav3p3vo5CR1sL8sAyPgf4cCEG2xoNkCA3Gg1ZcffpBTQSgvMxPHj W569z9C+7mUy2L0mfGU7Z25Q3CBSLohThEETjiCvd5b90DypwKe/GOJa87RFRj1X2Lo4qSyI +JTyvD3Kvocm1hW9YF7Ft5WIWgWvrMDfpdWkVZpGmvldVOuBu8yK3WKx5AX5KZM2qVYqU29X UfWootWPrCAOcXEFl8NJVV6MrTfhK9MwjSCv+4oJEja5TNs+ObVW0tlIBTR2jdWK6F4Md15z L556tIW8QG2ljEjLs2C0nJP722JI3FZC/cnu5gWDZXFkA0uzl0eM5XQBjWvuMOEaslWM1lsK TiR3fKQi7NZz0vEUnwyCXmSgrYN2cVQ4EhHlQZQKU6Il9zJgu4M8CdQqTlnHB5Iyhhn0v5oP jQ5PUNCOqjTrSxjg9JOXj7wFlgZVgGZ4EH413AAiHbdExuzTmXIIWAwZbSN8UQe/z4OdzRX5 ujFmmPsUDKsd8DtxCoiH0VirqW7H9B28wTDnuGhHtiEQMZmMWa03Pf2aDpasQbjDOMwmFbD9 Ltj8+tHYKHmMTId/v8gAI6A2LVMEB2JKQSumx26EH/lyY0ERAyP5A==
  • Ironport-hdrordr: A9a23:ds1z7agS1QWb2zb8kpq1NYjGcXBQXt4ji2hC6mlwRA09TyX+rb HIoB17726RtN91YhodcL+7VJVoLUmyyXcX2+ks1NWZMjUO0VHAROsO0WKI+VzdMhy72ulB1b pxN4hSYeeAaGSSVPyKgzVQxexQouW6zA==
  • Ironport-sdr: mq9R4FaZzdTbEtnsE3P+89x0bjuBafmuqxdHkh/B+P9AYHeNuXNAC5x2IWFNlD1rLVrggbWhrU 4HHnilt/MHwZyHsVgMwDSsHTrbHrRHVb6t7xnou/f8EcH4TpFuE8ZUGy8o1qI/yuZCMASYfL4w hVJuBVzwrQuQ8NkIBlf121RotjTtbI89mDSu4/JL0trLExnIanZiekTDyG+t/TuIQHJBtVWbvx YXsYo6ESGk4llfRFpjrml41ryMJrWbkXn+EGqfOqvj3rj6R9OMInMc41gr42kbs2jbiMyK1eJM wxcf37tom42gMTFUiZ22ntyY
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Nov 25, 2021 at 01:02:45PM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> Take into account guest's BAR view and program its p2m accordingly:
> gfn is guest's view of the BAR and mfn is the physical BAR value as set
> up by the PCI bus driver in the hardware domain.
> This way hardware domain sees physical BAR values and guest sees
> emulated ones.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> ---
> Since v4:
> - moved start_{gfn|mfn} calculation into map_range
> - pass vpci_bar in the map_data instead of start_{gfn|mfn}
> - s/guest_addr/guest_reg
> Since v3:
> - updated comment (Roger)
> - removed gfn_add(map->start_gfn, rc); which is wrong
> - use v->domain instead of v->vpci.pdev->domain
> - removed odd e.g. in comment
> - s/d%d/%pd in altered code
> - use gdprintk for map/unmap logs
> Since v2:
> - improve readability for data.start_gfn and restructure ?: construct
> Since v1:
>  - s/MSI/MSI-X in comments
> 
> ---
> ---
>  xen/drivers/vpci/header.c | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index cc49aa68886f..b0499d32c5d8 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -30,6 +30,7 @@
>  
>  struct map_data {
>      struct domain *d;
> +    const struct vpci_bar *bar;
>      bool map;
>  };
>  
> @@ -41,8 +42,25 @@ static int map_range(unsigned long s, unsigned long e, 
> void *data,
>  
>      for ( ; ; )
>      {
> +        /* Start address of the BAR as seen by the guest. */
> +        gfn_t start_gfn = _gfn(PFN_DOWN(is_hardware_domain(map->d)
> +                                        ? map->bar->addr
> +                                        : map->bar->guest_reg));
> +        /* Physical start address of the BAR. */
> +        mfn_t start_mfn = _mfn(PFN_DOWN(map->bar->addr));
>          unsigned long size = e - s + 1;
>  
> +        /*
> +         * Ranges to be mapped don't always start at the BAR start address, 
> as
> +         * there can be holes or partially consumed ranges. Account for the
> +         * offset of the current address from the BAR start.
> +         */
> +        start_gfn = gfn_add(start_gfn, s - mfn_x(start_mfn));

When doing guests mappings the rangeset should represent the guest
physical memory space, not the host one. So that collisions in the
guest p2m can be avoided. Also a guest should be allowed to map the
same mfn into multiple gfn. For example multiple BARs could share the
same physical page on the host and the guest might like to map them at
different pages in it's physmap.

> +
> +        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.

>          /*
>           * ARM TODOs:
>           * - On ARM whether the memory is prefetchable or not should be 
> passed
> @@ -52,8 +70,10 @@ static int map_range(unsigned long s, unsigned long e, 
> void *data,
>           * - {un}map_mmio_regions doesn't support preemption.
>           */
>  
> -        rc = map->map ? map_mmio_regions(map->d, _gfn(s), size, _mfn(s))
> -                      : unmap_mmio_regions(map->d, _gfn(s), size, _mfn(s));
> +        rc = map->map ? map_mmio_regions(map->d, start_gfn,
> +                                         size, _mfn(s))
> +                      : unmap_mmio_regions(map->d, start_gfn,
> +                                           size, _mfn(s));
>          if ( rc == 0 )
>          {
>              *c += size;
> @@ -62,8 +82,8 @@ static int map_range(unsigned long s, unsigned long e, void 
> *data,
>          if ( rc < 0 )
>          {
>              printk(XENLOG_G_WARNING
> -                   "Failed to identity %smap [%lx, %lx] for d%d: %d\n",
> -                   map->map ? "" : "un", s, e, map->d->domain_id, rc);
> +                   "Failed to identity %smap [%lx, %lx] for %pd: %d\n",
> +                   map->map ? "" : "un", s, e, map->d, rc);

You need to adjust the message here, as this is no longer an identity
map for domUs.

Thanks, Roger.



 


Rackspace

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