Re: [Qemu-devel] [RFC PATCH 2/3] hw/vfio: amba device support

[Eric Auger <eric.auger@linaro.org>]

On 12/17/2014 11:09 AM, Baptiste Reynal wrote:
> Add VFIO_DEVICE_TYPE_AMBA.
> Differentiate amba and platform devices according to compatible string.
> If the device is amba, clocks description is added in the fd.
> 
> Signed-off-by: Baptiste Reynal <b.reynal@virtualopensystems.com>
> ---
>  hw/arm/sysbus-fdt.c           | 11 +++++++++++
>  hw/vfio/platform.c            | 15 ++++++++++++---
>  include/hw/vfio/vfio-common.h |  1 +
>  3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index 125ba37..41b4c87 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -62,6 +62,8 @@ typedef struct NodeCreationPair {
>   *
>   * Generates a very simple node with following properties:
>   * compatible string, regs, interrupts
> + * And the following properties if it is on an AMBA bus :
> + * clocks, clock-names
>   */
>  static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
>  {
> @@ -103,6 +105,15 @@ static int add_generic_fdt_node(SysBusDevice *sbdev, void *opaque)
>      qemu_fdt_setprop(fdt, nodename, "compatible",
>                            compat, compat_str_len);
>  
> +    if (vbasedev->type == VFIO_DEVICE_TYPE_AMBA) {
> +        /* Setup clocks for AMBA device */
> +        qemu_fdt_setprop_cells(fdt, nodename, "clocks",
> +                qemu_fdt_getprop_cell(fdt, "/apb-pclk", "phandle"));
Shouldn't you test if this clock node exists in the guest device tree?
> +        char clock_names[] = "apb_pclk";
> +        qemu_fdt_setprop(fdt, nodename, "clock-names", clock_names,
> +                sizeof(clock_names));
> +    }
> +
>      reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
>  
>      for (i = 0; i < vbasedev->num_regions; i++) {
> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 39eef08..0a96178 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -563,8 +563,13 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
>      }
>  
>      /* Check that the host device exists */
> -    snprintf(path, sizeof(path), "/sys/bus/platform/devices/%s/",
> -             vbasedev->name);
> +    if (vbasedev->type == VFIO_DEVICE_TYPE_AMBA) {
> +        snprintf(path, sizeof(path), "/sys/bus/amba/devices/%s/",
> +                vbasedev->name);
> +    } else {
> +        snprintf(path, sizeof(path), "/sys/bus/platform/devices/%s/",
> +                vbasedev->name);
> +    }
>  
>      if (stat(path, &st) < 0) {
>          error_report("vfio: error: no such host device: %s", path);
> @@ -661,7 +666,11 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp)
>      VFIODevice *vbasedev = &vdev->vbasedev;
>      int i, ret;
>  
> -    vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
> +    if (strstr(vdev->compat, "arm,primecell")) {
> +        vbasedev->type = VFIO_DEVICE_TYPE_AMBA;
> +    } else {
> +        vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
> +    }
>      vbasedev->ops = &vfio_platform_ops;
The plan is to use VFIO_DEVICE_GET_INFO instead. Alex recently requested
we test the flags. We need a header sync for that.

Best Regards

Eric
>  
>  #ifdef CONFIG_KVM
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 58fd786..2f1b09c 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -49,6 +49,7 @@ extern int vfio_kvm_device_fd;
>  enum {
>      VFIO_DEVICE_TYPE_PCI = 0,
>      VFIO_DEVICE_TYPE_PLATFORM = 1,
> +    VFIO_DEVICE_TYPE_AMBA = 2,
>  };
>  
>  typedef struct VFIORegion {
>