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

RE: [PATCH V6 2/2] libxl: Introduce basic virtio-mmio support on Arm


  • To: Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Henry Wang <Henry.Wang@xxxxxxx>
  • Date: Thu, 9 Dec 2021 06:34:56 +0000
  • Accept-language: zh-CN, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.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=fMXSeHiFxkBI/tUs0uRWbNjp7s1B6jHptdm1d0wrwXo=; b=TOznYA0ABUcIhjYH//UMDsXOBUoc0TlO3IZhcr3LkjmIcItJNS8zm0u1hAr5CdihFNJPdmEsgv3TnWbaDtL4iqOLjeDO6zj9khB9xMAbXXptLVtmwlcADysXMxr8rw29pLy3eqYRYoWIp+pRluLgeQSGBctzsiWs8LnsnE3Zi5+LcyEImcKV/kIiP1evoedb8EYeS/HUwJ4U65v1FSefxkAZ4ThOlnVj776Zgmz2+hsniDU235uYpypYYZAzkcmaaTqC8DHB0DR0Aq3JC6jWX+DAoCL8OgxQC/wIVE25GfOn1AXlUbYXtS8ZxNSYEhW1J8IGtO1qc2BaUlEOxCqThQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UENoAlvbW7Rvw7B2RQEDCXXSOrwPptYhZpA58lcANNJ9iMZhFPIgP0m5F5duqSOL8RqnS2qAMywZ4VOLYqebhFKMf4i5tzbPAaCmKfm0GtN1jdBIRtpOG26sjzG3oF7maeqzfT1j68w7NJSqxNmOiLEezMFbibqxUUns91U1X32SFXdDpDNjjZU31mI+toACicvkib7SP5drFANrf3B7n7dM+W6pm5Z0sDVmciKqhud4uXJSyEWyak5fS3ZP8fAUEe/TYjjPg0BMFVlrTMDSgXuv89oFDghq2LPCRvQ2Ka+kuhsTLEEiDNbJrduGOaSqNntKfUiZWDpS6toV+KBoQw==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Julien Grall <Julien.Grall@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
  • Delivery-date: Thu, 09 Dec 2021 06:35:25 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHX7FUn0czI/NY3iEeAQwXmMstuq6wptMyg
  • Thread-topic: [PATCH V6 2/2] libxl: Introduce basic virtio-mmio support on Arm

Hi Oleksandr,

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> Oleksandr Tyshchenko
> Sent: Thursday, December 9, 2021 1:00 AM
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Julien Grall <Julien.Grall@xxxxxxx>; Wei Liu <wl@xxxxxxx>; Anthony
> PERARD <anthony.perard@xxxxxxxxxx>; Juergen Gross <jgross@xxxxxxxx>;
> Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>;
> Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; Bertrand Marquis
> <Bertrand.Marquis@xxxxxxx>; Oleksandr Tyshchenko
> <oleksandr_tyshchenko@xxxxxxxx>
> Subject: [PATCH V6 2/2] libxl: Introduce basic virtio-mmio support on Arm
> 
> From: Julien Grall <julien.grall@xxxxxxx>
> 
> This patch introduces helpers to allocate Virtio MMIO params
> (IRQ and memory region) and create specific device node in
> the Guest device-tree with allocated params. In order to deal
> with multiple Virtio devices, reserve corresponding ranges.
> For now, we reserve 1MB for memory regions and 10 SPIs.
> 
> As these helpers should be used for every Virtio device attached
> to the Guest, call them for Virtio disk(s).
> 
> Please note, with statically allocated Virtio IRQs there is
> a risk of a clash with a physical IRQs of passthrough devices.
> For the first version, it's fine, but we should consider allocating
> the Virtio IRQs automatically. Thankfully, we know in advance which
> IRQs will be used for passthrough to be able to choose non-clashed
> ones.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> ---
> Please note, this is a split/cleanup/hardening of Julien's PoC:
> "Add support for Guest IO forwarding to a device emulator"
> 
> Changes RFC -> V1:
>    - was squashed with:
>      "[RFC PATCH V1 09/12] libxl: Handle virtio-mmio irq in more correct way"
>      "[RFC PATCH V1 11/12] libxl: Insert "dma-coherent" property into virtio-
> mmio device node"
>      "[RFC PATCH V1 12/12] libxl: Fix duplicate memory node in DT"
>    - move VirtIO MMIO #define-s to xen/include/public/arch-arm.h
> 
> Changes V1 -> V2:
>    - update the author of a patch
> 
> Changes V2 -> V3:
>    - no changes
> 
> Changes V3 -> V4:
>    - no changes
> 
> Changes V4 -> V5:
>    - split the changes, change the order of the patches
>    - drop an extra "virtio" configuration option
>    - update patch description
>    - use CONTAINER_OF instead of own implementation
>    - reserve ranges for Virtio MMIO params and put them
>      in correct location
>    - create helpers to allocate Virtio MMIO params, add
>      corresponding sanity-сhecks
>    - add comment why MMIO size 0x200 is chosen
>    - update debug print
>    - drop Wei's T-b
> 
> Changes V5 -> V6:
>    - rebase on current staging
> ---
>  tools/libs/light/libxl_arm.c  | 131
> +++++++++++++++++++++++++++++++++++++++++-
>  xen/include/public/arch-arm.h |   7 +++
>  2 files changed, 136 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index eef1de0..d475249 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -8,6 +8,56 @@
>  #include <assert.h>
>  #include <xen/device_tree_defs.h>
> 
> +/*
> + * There is no clear requirements for the total size of Virtio MMIO region.
> + * The size of control registers is 0x100 and device-specific configuration
> + * registers starts at the offset 0x100, however it's size depends on the
> device
> + * and the driver. Pick the biggest known size at the moment to cover most
> + * of the devices (also consider allowing the user to configure the size via
> + * config file for the one not conforming with the proposed value).
> + */
> +#define VIRTIO_MMIO_DEV_SIZE   xen_mk_ullong(0x200)
> +
> +static uint64_t virtio_mmio_base;
> +static uint32_t virtio_mmio_irq;
> +
> +static void init_virtio_mmio_params(void)
> +{
> +    virtio_mmio_base = GUEST_VIRTIO_MMIO_BASE;
> +    virtio_mmio_irq = GUEST_VIRTIO_MMIO_SPI_FIRST;
> +}
> +
> +static uint64_t alloc_virtio_mmio_base(libxl__gc *gc)
> +{
> +    uint64_t base = virtio_mmio_base;
> +
> +    /* Make sure we have enough reserved resources */
> +    if ((virtio_mmio_base + VIRTIO_MMIO_DEV_SIZE >
> +        GUEST_VIRTIO_MMIO_BASE + GUEST_VIRTIO_MMIO_SIZE)) {
> +        LOG(ERROR, "Ran out of reserved range for Virtio MMIO BASE
> 0x%"PRIx64"\n",
> +            virtio_mmio_base);
> +        return 0;
> +    }
> +    virtio_mmio_base += VIRTIO_MMIO_DEV_SIZE;
> +
> +    return base;
> +}
> +
> +static uint32_t alloc_virtio_mmio_irq(libxl__gc *gc)
> +{
> +    uint32_t irq = virtio_mmio_irq;
> +
> +    /* Make sure we have enough reserved resources */
> +    if (virtio_mmio_irq > GUEST_VIRTIO_MMIO_SPI_LAST) {
> +        LOG(ERROR, "Ran out of reserved range for Virtio MMIO IRQ %u\n",
> +            virtio_mmio_irq);
> +        return 0;
> +    }
> +    virtio_mmio_irq++;
> +
> +    return irq;
> +}
> +
>  static const char *gicv_to_string(libxl_gic_version gic_version)
>  {
>      switch (gic_version) {
> @@ -26,8 +76,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>  {
>      uint32_t nr_spis = 0;
>      unsigned int i;
> -    uint32_t vuart_irq;
> -    bool vuart_enabled = false;
> +    uint32_t vuart_irq, virtio_irq = 0;
> +    bool vuart_enabled = false, virtio_enabled = false;
> 
>      /*
>       * If pl011 vuart is enabled then increment the nr_spis to allow 
> allocation
> @@ -39,6 +89,35 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>          vuart_enabled = true;
>      }
> 
> +    /*
> +     * Virtio MMIO params are non-unique across the whole system and
> must be
> +     * initialized for every new guest.
> +     */
> +    init_virtio_mmio_params();
> +    for (i = 0; i < d_config->num_disks; i++) {
> +        libxl_device_disk *disk = &d_config->disks[i];
> +
> +        if (disk->virtio) {
> +            disk->base = alloc_virtio_mmio_base(gc);
> +            if (!disk->base)
> +                return ERROR_FAIL;
> +
> +            disk->irq = alloc_virtio_mmio_irq(gc);
> +            if (!disk->irq)
> +                return ERROR_FAIL;
> +
> +            if (virtio_irq < disk->irq)
> +                virtio_irq = disk->irq;
> +            virtio_enabled = true;
> +
> +            LOG(DEBUG, "Allocate Virtio MMIO params for Vdev %s: IRQ %u
> BASE 0x%"PRIx64,
> +                disk->vdev, disk->irq, disk->base);
> +        }
> +    }
> +
> +    if (virtio_enabled)
> +        nr_spis += (virtio_irq - 32) + 1;
> +
>      for (i = 0; i < d_config->b_info.num_irqs; i++) {
>          uint32_t irq = d_config->b_info.irqs[i];
>          uint32_t spi;
> @@ -58,6 +137,13 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>              return ERROR_FAIL;
>          }
> 
> +        /* The same check as for vpl011 */
> +        if (virtio_enabled &&
> +           (irq >= GUEST_VIRTIO_MMIO_SPI_FIRST && irq <= virtio_irq)) {
> +            LOG(ERROR, "Physical IRQ %u conflicting with Virtio MMIO IRQ
> range\n", irq);
> +            return ERROR_FAIL;
> +        }
> +
>          if (irq < 32)
>              continue;
> 
> @@ -787,6 +873,39 @@ static int make_vpci_node(libxl__gc *gc, void *fdt,
>      return 0;
>  }
> 
> +
> +static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
> +                                 uint64_t base, uint32_t irq)
> +{
> +    int res;
> +    gic_interrupt intr;
> +    /* Placeholder for virtio@ + a 64-bit number + \0 */
> +    char buf[24];
> +
> +    snprintf(buf, sizeof(buf), "virtio@%"PRIx64, base);
> +    res = fdt_begin_node(fdt, buf);
> +    if (res) return res;
> +
> +    res = fdt_property_compat(gc, fdt, 1, "virtio,mmio");
> +    if (res) return res;
> +
> +    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> GUEST_ROOT_SIZE_CELLS,
> +                            1, base, VIRTIO_MMIO_DEV_SIZE);
> +    if (res) return res;
> +
> +    set_interrupt(intr, irq, 0xf, DT_IRQ_TYPE_EDGE_RISING);
> +    res = fdt_property_interrupts(gc, fdt, &intr, 1);
> +    if (res) return res;
> +
> +    res = fdt_property(fdt, "dma-coherent", NULL, 0);
> +    if (res) return res;
> +
> +    res = fdt_end_node(fdt);
> +    if (res) return res;
> +
> +    return 0;
> +}
> +
>  static const struct arch_info *get_arch_info(libxl__gc *gc,
>                                               const struct xc_dom_image *dom)
>  {
> @@ -988,6 +1107,7 @@ static int libxl__prepare_dtb(libxl__gc *gc,
> libxl_domain_config *d_config,
>      size_t fdt_size = 0;
>      int pfdt_size = 0;
>      libxl_domain_build_info *const info = &d_config->b_info;
> +    unsigned int i;
> 
>      const libxl_version_info *vers;
>      const struct arch_info *ainfo;
> @@ -1094,6 +1214,13 @@ next_resize:
>          if (d_config->num_pcidevs)
>              FDT( make_vpci_node(gc, fdt, ainfo, dom) );
> 
> +        for (i = 0; i < d_config->num_disks; i++) {
> +            libxl_device_disk *disk = &d_config->disks[i];
> +
> +            if (disk->virtio)
> +                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) );
> +        }
> +
>          if (pfdt)
>              FDT( copy_partial_fdt(gc, fdt, pfdt) );
> 
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 94b3151..6dc55df 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -398,6 +398,10 @@ typedef uint64_t xen_callback_t;
> 
>  /* Physical Address Space */
> 
> +/* Virtio MMIO mappings */
> +#define GUEST_VIRTIO_MMIO_BASE   xen_mk_ullong(0x02000000)
> +#define GUEST_VIRTIO_MMIO_SIZE   xen_mk_ullong(0x00100000)
> +
>  /*
>   * vGIC mappings: Only one set of mapping is used by the guest.
>   * Therefore they can overlap.
> @@ -484,6 +488,9 @@ typedef uint64_t xen_callback_t;
> 
>  #define GUEST_VPL011_SPI        32
> 
> +#define GUEST_VIRTIO_MMIO_SPI_FIRST   33
> +#define GUEST_VIRTIO_MMIO_SPI_LAST    43
> +
>  /* PSCI functions */
>  #define PSCI_cpu_suspend 0
>  #define PSCI_cpu_off     1
> --
> 2.7.4
> 

Reviewed-by: Henry Wang <Henry.Wang@xxxxxxx>

Kind regards,

Henry

 


Rackspace

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