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

Re: [PATCH v5 12/14] xen/arm: translate virtual PCI bus topology for guests


  • To: Oleksandr Andrushchenko <andr2000@xxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 13 Jan 2022 13:18:08 +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=XE3/eOH5mYVV5j3s61hJ7bU+F4KZdw59sbl8x8LH/cQ=; b=OXbs/UHCWboQZvQWbDj1jYDvpylu31AEmLTNxe/oiAONeCa57TmSG/vV3RRcEiFtlrIm/dF58bYGiLre247ZM/j7C6B3YfjFTzL0c52KzgPZmMBZipDUgATqogEOKxFRLwTRVTsL+vP7iOiubAcRk+eymzqQHEzXoshLMNHA7k6WgEpJrf7Z0GZSMaIqDSwDTvQWmdrb4rnIk79qk1oa230iZ6Oi4gWY3XIuWYL2CRnCVsHx2EfQ7pCQzGLG4OHazd7cKgqGtZ+9wvtWYb8hbVdYwChYGwXmbEgHauEnhJsAcmB0GK8PTxCCIAgvGKL3f+qmXgenbuHv3jOv4D4Avw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JeLyqlA/zAzv8lepbdeIOu8QWCGbYLyc/rQnYGyJrHP/G6ANV1DXcFVUlZxkThp2kUo0m4I2feXXKEy7Rpq7wglRIL/IY8HH+rpwUQrus3u/6Kj9KqcIwFFO7OIyX6hwbc8DDBdERZE8CNfA4XGrOmRlzkr+z330N+bt5Zc2AMP9NSEZR4iTuSIQyALeUlwCM0FOrltx5c4h8xHik+biqTp60d8hdp/0D709VCGkcKZU25iyjZLB8R0fMjguOdCvPlLvIyWuf4I4ECo6nZrrZqvetmHmb37bDHqjeW3K09FOUv3Y124LJdkpztTeXpCsOxH5DwecSsS5yUZ09vlnIA==
  • Authentication-results: esa1.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 12:18:38 +0000
  • Ironport-data: A9a23:nGu/8q9XrObz0PUEumSqDrUDQHiTJUtcMsCJ2f8bNWPcYEJGY0x3y mAcDGmAPf6PYDH0e993bYni9xhV7J/cm9RhSlRsqSs8E34SpcT7XtnIdU2Y0wF+jyHgoOCLy +1EN7Es+ehtFie0Si9AttENlFEkvU2ybuOU5NXsZ2YhFWeIdA970Ug5w7dg2dYy6TSEK1jlV e3a8pW31GCNg1aYAkpMg05UgEoy1BhakGpwUm0WPZinjneH/5UmJMt3yZWKB2n5WuFp8tuSH I4v+l0bElTxpH/BAvv9+lryn9ZjrrT6ZWBigVIOM0Sub4QrSoXfHc/XOdJFAXq7hQllkPhe2 dJ8n5aXYD0uO6f2pr84dCkGEQBXaPguFL/veRBTsOSWxkzCNXDt3+9vHAc9OohwFuRfWD8Us 6ZCcXZUM07F17neLLGTE4GAguw5K8bmJsUHs2xIxjDFF/c2B5vERs0m4PcGhmxp2Z0TR54yY eIXQ2czbA/RMiYMEQZLKp0DpMmGtEXwJmgwRFW9+vNsvjm7IBZK+KDkLd79atGMA8JPkS6wp GjL4mD4CREyL8GExHyO9XfErsbVgS7+b6cDG7S5++BCjUWawyoYDxh+fVy/rPqig0iyQeVWL UAO5zEupqg/8k+sZtTlVhj+q3mB1jYGUtpNF6sh6QeCyoLd+QPfDW8BJhZLZcY6rsYwSXov3 0WQgtLyLTV1tfueTnf13rWJqTK/PwAFIGlEYjULJSMV7t+mrIwtgxbnStd4DLXzntDzASv3w T2BsG45nbp7pccN27/hpQifqz2pr5nNCAUy423/QGWh6Q9oYZ+/UIah41Pb8PVoIZ6QSx+Ku 31ss8+a4eMVBJeBjhuRUf4NF7Gk4fWCGDDEiFspFJ4knxy24GKqd41U5DB4JW9qP9wCdDuvZ 1Xc0T69/7cKYiHsN/UuJdvsVYJ6lsAMCOgJSNj7f91/SJJWXTWA+StUWxW1+FCzkmkFxPRX1 YigTe6gCnMTCKJCxTWwRvsA3bJD+h3S1V8/VrigkU35jOP2iGq9DO5cbQDQNrxRALas/V2Nm +uzIfdm3Pm2vAfWRiDMubAeIlkRRZTQLcCn8pcHHgJvz+cPJY3ANxMz6e9wE2CGt/4M/gstw p1bchUIoLYYrSeWQThmklg5NNvSsW9X9BrXxxAEM1eywGQEao2y9qoZfJZfVeB5qLY7lKUsE aVVIJ/o7hFzptLvoWR1gX7V9t0KSfhWrVjWY3rNjMYXIvaMuDAlCve7J1CypUHi/wK8tNcko q3I6+8oacFreuiWN+6PMKjH5wro5RA1wbsuN2OVfIU7UBiyoeBCdnyg5tdqcppkAUiSmVOnO /O+XE1wSR/l+dFlqbEkRMms8u+ULgeJNhELQDmAs+fnbHmyE6jK6dYobdtktAv1DQvc0K6je f9U37f7NvgGl0xNqI1yD/BgyqdW2jclj+YyIt1MECqZYlK1JKlnJ3Xaj8BDurcUnu1SuBesW 1LJ8d5fYO3bNMTgGV8XBQwkcuXciq1ExmiMtaw4cBfg+St63LubSkEObROCvzNQceluO4Q/z OZ/5MNPs16jigAnO8qthzxP8zjeNWQJVqgq78lIAILihgcx5EtFZJjQVn3/7J2VMo0eOUg2O D6EwqHFgu0ElEbFdnMyE1nL3PZc2stS6EwbkgdaKg3QyNTfh/Ix0BlAyhgNT1xYnkddzuZ+G ml3LEkpd6+AyChl2ZpYVGe2FgAfWBDAoh7ty0EEnXHyRlWzUjCfN3U0POuA8RxL829YeTQHr riUxHy8DGTvdcD1mCAzRVRku7roStkorl/On8WuHsKkGZgmYGW63v/yNDRQ8xa3U9ksgEDnp PVx+LciYKL2AiccvqknBtTIzr8XUh2FeDRPTPwJEHnlxo0AlOVeAQSzFn0=
  • Ironport-hdrordr: A9a23:eRy5BaiVC1ljOeLG7V7eWRZyvXBQXzh13DAbv31ZSRFFG/FwyP rAoB1L73PJYWgqNU3I+ergBEGBKUmskqKdxbNhR4tKPTOWw1dASbsN0WKM+UyDJ8STzJ856U 4kSdkCNDSSNykFsS+Z2njALz9I+rDum8rJ9ITjJjVWPHlXgslbnnhE422gYytLrWd9dP4E/M 323Ls6m9PsQwVeUu2LQl0+G8TTrdzCk5zrJTYAGh4c8QGLyRel8qTzHRS01goXF2on+8ZpzU H11yjCoomzufCyzRHRk0fV8pRtgdPkjv9OHtaFhMQ5IijlziyoeINicbufuy1dmpDl1H8a1P 335zswNcV67H3cOkmzvBvWwgHllA0j7nfzoGXo9kfLkIjcfnYXGsBBjYVWfl/y8Ew7puxx16 pNwiawq4dXJQmoplWz2/H4EzVR0makq3srluAey1ZFV5EFVbNXpYsDuGtIDZY7Gj7g4oxPKp gjMCjl3ocWTbqmVQGYgoE2q+bcHUjbXy32D3Tqg/blnQS/xxtCvgklLM92pAZ1yHtycegA2w 3+CNUZqFh5dL5iUUtMPpZxfSKJMB2/ffvtChPlHb21LtBPB5ryw6SHkondotvaPKA18A==
  • Ironport-sdr: yWPlkplV1nfc8Ev14kGzXunBnPww4w+96ll7IbWlJcBzmIpA0hh01AkuxYud2LW3Wz4HGLIw1i FSj/kyqYLZgjCBlBbTxH/WLKHDG2V8W9bgV9AMUowv8iPmIMB8uCS9DmUkrnGCQPsSa7gy2yPP kZIZ+fmDqAqgPN1imLUNPrxLFk7WSfpO7yRYgP6CuiQ3vxh5KwcQD3x6oe+G5JDFgMbz1BhTaq gWmVGMh8pIqF0OsCaB8u4JVTYZGKfCffqsVliGWVcJgQj68h3zejqNxfGFrGPMDe5TUIK+ZR5z T6engfZq/oqqywQQYFn8uQgJ
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Nov 25, 2021 at 01:02:49PM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> There are three  originators for the PCI configuration space access:
> 1. The domain that owns physical host bridge: MMIO handlers are
> there so we can update vPCI register handlers with the values
> written by the hardware domain, e.g. physical view of the registers
> vs guest's view on the configuration space.
> 2. Guest access to the passed through PCI devices: we need to properly
> map virtual bus topology to the physical one, e.g. pass the configuration
> space access to the corresponding physical devices.
> 3. Emulated host PCI bridge access. It doesn't exist in the physical
> topology, e.g. it can't be mapped to some physical host bridge.
> So, all access to the host bridge itself needs to be trapped and
> emulated.

I'm kind of lost in this commit message. You are just adding a
translate function in order for domUs to translate from virtual SBDF
to the physical SBDF of the device. I realize you do that based on
whether 'bridge' is set or not, so I assume this is just a way to
signal whether the domain is a hardware domain or not. Ie:
!!bridge == is_hardware_domain(v->domain).

> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> ---
> Since v4:
> - indentation fixes
> - constify struct domain
> - updated commit message
> - updates to the new locking scheme (pdev->vpci_lock)
> Since v3:
> - revisit locking
> - move code to vpci.c
> Since v2:
>  - pass struct domain instead of struct vcpu
>  - constify arguments where possible
>  - gate relevant code with CONFIG_HAS_VPCI_GUEST_SUPPORT
> New in v2
> ---
>  xen/arch/arm/vpci.c     | 18 ++++++++++++++++++
>  xen/drivers/vpci/vpci.c | 27 +++++++++++++++++++++++++++
>  xen/include/xen/vpci.h  |  1 +
>  3 files changed, 46 insertions(+)
> 
> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
> index 8e801f275879..3d134f42d07e 100644
> --- a/xen/arch/arm/vpci.c
> +++ b/xen/arch/arm/vpci.c
> @@ -41,6 +41,15 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t 
> *info,
>      /* data is needed to prevent a pointer cast on 32bit */
>      unsigned long data;
>  
> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
> +    /*
> +     * For the passed through devices we need to map their virtual SBDF
> +     * to the physical PCI device being passed through.
> +     */
> +    if ( !bridge && !vpci_translate_virtual_device(v->domain, &sbdf) )
> +        return 1;

I'm unsure what returning 1 implies for Arm here, but you likely need
to set '*r = ~0ul;'.

> +#endif
> +
>      if ( vpci_ecam_read(sbdf, ECAM_REG_OFFSET(info->gpa),
>                          1U << info->dabt.size, &data) )
>      {
> @@ -59,6 +68,15 @@ static int vpci_mmio_write(struct vcpu *v, mmio_info_t 
> *info,
>      struct pci_host_bridge *bridge = p;
>      pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
>  
> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
> +    /*
> +     * For the passed through devices we need to map their virtual SBDF
> +     * to the physical PCI device being passed through.
> +     */
> +    if ( !bridge && !vpci_translate_virtual_device(v->domain, &sbdf) )
> +        return 1;
> +#endif
> +
>      return vpci_ecam_write(sbdf, ECAM_REG_OFFSET(info->gpa),
>                             1U << info->dabt.size, r);
>  }
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index c2fb4d4db233..bdc8c63f73fa 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -195,6 +195,33 @@ static void vpci_remove_virtual_device(struct domain *d,
>      pdev->vpci->guest_sbdf.sbdf = ~0;
>  }
>  
> +/*
> + * Find the physical device which is mapped to the virtual device
> + * and translate virtual SBDF to the physical one.
> + */
> +bool vpci_translate_virtual_device(const struct domain *d, pci_sbdf_t *sbdf)
> +{
> +    struct pci_dev *pdev;
> +

I would add:

ASSERT(!is_hardware_domain(d));

To make sure this is not used for the hardware domain.

> +    for_each_pdev( d, pdev )
> +    {
> +        bool found;
> +
> +        spin_lock(&pdev->vpci_lock);
> +        found = pdev->vpci && (pdev->vpci->guest_sbdf.sbdf == sbdf->sbdf);
> +        spin_unlock(&pdev->vpci_lock);
> +
> +        if ( found )
> +        {
> +            /* Replace guest SBDF with the physical one. */
> +            *sbdf = pdev->sbdf;
> +            return true;
> +        }
> +    }
> +
> +    return false;
> +}
> +
>  /* Notify vPCI that device is assigned to guest. */
>  int vpci_assign_device(struct domain *d, struct pci_dev *pdev)
>  {
> diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
> index e5258bd7ce90..21d76929391f 100644
> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -280,6 +280,7 @@ static inline void vpci_cancel_pending_locked(struct 
> pci_dev *pdev)
>  /* Notify vPCI that device is assigned/de-assigned to/from guest. */
>  int vpci_assign_device(struct domain *d, struct pci_dev *pdev);
>  int vpci_deassign_device(struct domain *d, struct pci_dev *pdev);
> +bool vpci_translate_virtual_device(const struct domain *d, pci_sbdf_t *sbdf);
>  #else
>  static inline int vpci_assign_device(struct domain *d, struct pci_dev *pdev)
>  {

If you add a dummy vpci_translate_virtual_device helper that returns
false unconditionally here you could drop the #ifdefs in arm/vpci.c
AFAICT.

Thanks, Roger.



 


Rackspace

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