Re: [PATCH v2 7/9] ALSA: virtio: introduce jack support

[Guennadi Liakhovetski]

On Sun, 24 Jan 2021, Anton Yakovlev wrote:


Enumerate all available jacks and create ALSA controls.

At the moment jacks have a simple implementation and can only be used
to receive notifications about a plugged in/out device.

Signed-off-by: Anton Yakovlev 
---
sound/virtio/Makefile  |   1 +
sound/virtio/virtio_card.c |  18 +++
sound/virtio/virtio_card.h |  12 ++
sound/virtio/virtio_jack.c | 255 +
4 files changed, 286 insertions(+)
create mode 100644 sound/virtio/virtio_jack.c


[snip]


diff --git a/sound/virtio/virtio_jack.c b/sound/virtio/virtio_jack.c
new file mode 100644
index ..83593c59f6bf
--- /dev/null
+++ b/sound/virtio/virtio_jack.c
@@ -0,0 +1,255 @@


[snip]


+/**
+ * virtsnd_jack_parse_cfg() - Parse the jack configuration.
+ * @snd: VirtIO sound device.
+ *
+ * This function is called during initial device initialization.
+ *
+ * Context: Any context that permits to sleep.
+ * Return: 0 on success, -errno on failure.
+ */
+int virtsnd_jack_parse_cfg(struct virtio_snd *snd)
+{
+   struct virtio_device *vdev = snd->vdev;
+   struct virtio_snd_jack_info *info;
+   unsigned int i;
+   int rc;
+
+   virtio_cread(vdev, struct virtio_snd_config, jacks, >njacks);
+   if (!snd->njacks)
+   return 0;
+
+   snd->jacks = devm_kcalloc(>dev, snd->njacks, sizeof(*snd->jacks),
+ GFP_KERNEL);
+   if (!snd->jacks)
+   return -ENOMEM;
+
+   info = devm_kcalloc(>dev, snd->njacks, sizeof(*info), GFP_KERNEL);


just kcalloc()


+   if (!info)
+   return -ENOMEM;
+
+   rc = virtsnd_ctl_query_info(snd, VIRTIO_SND_R_JACK_INFO, 0, snd->njacks,
+   sizeof(*info), info);
+   if (rc)
+   return rc;
+
+   for (i = 0; i < snd->njacks; ++i) {
+   struct virtio_jack *jack = >jacks[i];
+   struct virtio_pcm *pcm;
+
+   jack->nid = le32_to_cpu(info[i].hdr.hda_fn_nid);
+   jack->features = le32_to_cpu(info[i].features);
+   jack->defconf = le32_to_cpu(info[i].hda_reg_defconf);
+   jack->caps = le32_to_cpu(info[i].hda_reg_caps);
+   jack->connected = info[i].connected;
+
+   pcm = virtsnd_pcm_find_or_create(snd, jack->nid);
+   if (IS_ERR(pcm))
+   return PTR_ERR(pcm);
+   }
+
+   devm_kfree(>dev, info);
+
+   return 0;
+}


Thanks
Guennadi
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 6/9] ALSA: virtio: PCM substream operators

[Guennadi Liakhovetski]

One more thing I missed yesterday:

On Mon, 25 Jan 2021, Guennadi Liakhovetski wrote:



On Sun, 24 Jan 2021, Anton Yakovlev wrote:


Introduce the operators required for the operation of substreams.

Signed-off-by: Anton Yakovlev 
---
sound/virtio/Makefile |   3 +-
sound/virtio/virtio_pcm.c |   5 +-
sound/virtio/virtio_pcm.h |   2 +
sound/virtio/virtio_pcm_ops.c | 513 ++
4 files changed, 521 insertions(+), 2 deletions(-)
create mode 100644 sound/virtio/virtio_pcm_ops.c


[snip]


diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
new file mode 100644
index ..19882777fcd6
--- /dev/null
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -0,0 +1,513 @@


[snip]


+/**
+ * virtsnd_pcm_release() - Release the PCM substream on the device side.
+ * @substream: VirtIO substream.
+ *
+ * Context: Any context that permits to sleep.
+ * Return: 0 on success, -errno on failure.
+ */
+static inline bool virtsnd_pcm_released(struct virtio_pcm_substream 
*substream)

+{
+   /*
+	 * The spec states that upon receipt of the RELEASE command "the 
device
+	 * MUST complete all pending I/O messages for the specified stream 
ID".

+* Thus, we consider the absence of I/O messages in the queue as an
+* indication that the substream has been released.
+*/
+   return atomic_read(>msg_count) == 0;


Also here having it atomic doesn't really seem to help. This just means, that 
at some point of time it was == 0.



+}
+
+static int virtsnd_pcm_release(struct virtio_pcm_substream *substream)


kernel-doc missing


+{
+   struct virtio_snd *snd = substream->snd;
+   struct virtio_snd_msg *msg;
+   unsigned int js = msecs_to_jiffies(msg_timeout_ms);
+   int rc;
+
+   msg = virtsnd_pcm_ctl_msg_alloc(substream, VIRTIO_SND_R_PCM_RELEASE,
+   GFP_KERNEL);
+   if (IS_ERR(msg))
+   return PTR_ERR(msg);
+
+   rc = virtsnd_ctl_msg_send_sync(snd, msg);
+   if (rc)
+   return rc;
+
+   return wait_event_interruptible_timeout(substream->msg_empty,
+   virtsnd_pcm_released(substream),
+   js);


wait_event_interruptible_timeout() will return a positive number in 
success cases, 0 means a timeout and condition still false. Whereas when 
you call this function you interpret 0 as success and you expect any != 0 
to be a negative error. Wondering how this worked during your tests?


Thanks
Guennadi
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH iproute2-next 2/2] vdpa: Add vdpa tool

[David Ahern]

Looks fine. A few comments below around code re-use.

On 1/22/21 4:26 AM, Parav Pandit wrote:
> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> new file mode 100644
> index ..942524b7
> --- /dev/null
> +++ b/vdpa/vdpa.c
> @@ -0,0 +1,828 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "mnl_utils.h"
> +
> +#include "version.h"
> +#include "json_print.h"
> +#include "utils.h"
> +
> +static int g_indent_level;
> +
> +#define INDENT_STR_STEP 2
> +#define INDENT_STR_MAXLEN 32
> +static char g_indent_str[INDENT_STR_MAXLEN + 1] = "";

The indent code has a lot of parallels with devlink -- including helpers
below around indent_inc and _dec. Please take a look at how to refactor
and re-use.

> +
> +struct vdpa_socket {
> + struct mnl_socket *nl;
> + char *buf;
> + uint32_t family;
> + unsigned int seq;
> +};
> +
> +static int vdpa_socket_sndrcv(struct vdpa_socket *nlg, const struct nlmsghdr 
> *nlh,
> +   mnl_cb_t data_cb, void *data)
> +{
> + int err;
> +
> + err = mnl_socket_sendto(nlg->nl, nlh, nlh->nlmsg_len);
> + if (err < 0) {
> + perror("Failed to send data");
> + return -errno;
> + }
> +
> + err = mnlu_socket_recv_run(nlg->nl, nlh->nlmsg_seq, nlg->buf, 
> MNL_SOCKET_BUFFER_SIZE,
> +data_cb, data);
> + if (err < 0) {
> + fprintf(stderr, "vdpa answers: %s\n", strerror(errno));
> + return -errno;
> + }
> + return 0;
> +}
> +
> +static int get_family_id_attr_cb(const struct nlattr *attr, void *data)
> +{
> + int type = mnl_attr_get_type(attr);
> + const struct nlattr **tb = data;
> +
> + if (mnl_attr_type_valid(attr, CTRL_ATTR_MAX) < 0)
> + return MNL_CB_ERROR;
> +
> + if (type == CTRL_ATTR_FAMILY_ID &&
> + mnl_attr_validate(attr, MNL_TYPE_U16) < 0)
> + return MNL_CB_ERROR;
> + tb[type] = attr;
> + return MNL_CB_OK;
> +}
> +
> +static int get_family_id_cb(const struct nlmsghdr *nlh, void *data)
> +{
> + struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
> + struct nlattr *tb[CTRL_ATTR_MAX + 1] = {};
> + uint32_t *p_id = data;
> +
> + mnl_attr_parse(nlh, sizeof(*genl), get_family_id_attr_cb, tb);
> + if (!tb[CTRL_ATTR_FAMILY_ID])
> + return MNL_CB_ERROR;
> + *p_id = mnl_attr_get_u16(tb[CTRL_ATTR_FAMILY_ID]);
> + return MNL_CB_OK;
> +}
> +
> +static int family_get(struct vdpa_socket *nlg)
> +{
> + struct genlmsghdr hdr = {};
> + struct nlmsghdr *nlh;
> + int err;
> +
> + hdr.cmd = CTRL_CMD_GETFAMILY;
> + hdr.version = 0x1;
> +
> + nlh = mnlu_msg_prepare(nlg->buf, GENL_ID_CTRL,
> +NLM_F_REQUEST | NLM_F_ACK,
> +, sizeof(hdr));
> +
> + mnl_attr_put_strz(nlh, CTRL_ATTR_FAMILY_NAME, VDPA_GENL_NAME);
> +
> + err = mnl_socket_sendto(nlg->nl, nlh, nlh->nlmsg_len);
> + if (err < 0)
> + return err;
> +
> + err = mnlu_socket_recv_run(nlg->nl, nlh->nlmsg_seq, nlg->buf,
> +MNL_SOCKET_BUFFER_SIZE,
> +get_family_id_cb, >family);
> + return err;
> +}
> +
> +static int vdpa_socket_open(struct vdpa_socket *nlg)
> +{
> + int err;
> +
> + nlg->buf = malloc(MNL_SOCKET_BUFFER_SIZE);
> + if (!nlg->buf)
> + goto err_buf_alloc;
> +
> + nlg->nl = mnlu_socket_open(NETLINK_GENERIC);
> + if (!nlg->nl)
> + goto err_socket_open;
> +
> + err = family_get(nlg);
> + if (err)
> + goto err_socket;
> +
> + return 0;
> +
> +err_socket:
> + mnl_socket_close(nlg->nl);
> +err_socket_open:
> + free(nlg->buf);
> +err_buf_alloc:
> + return -1;
> +}

The above 4 functions duplicate a lot of devlink functionality. Please
create a helper in lib/mnl_utils.c that can be used in both.

> +
> +static void vdpa_socket_close(struct vdpa_socket *nlg)
> +{
> + mnl_socket_close(nlg->nl);
> + free(nlg->buf);
> +}
> +
> +#define VDPA_OPT_MGMTDEV_HANDLE  BIT(0)
> +#define VDPA_OPT_VDEV_MGMTDEV_HANDLE BIT(1)
> +#define VDPA_OPT_VDEV_NAME   BIT(2)
> +#define VDPA_OPT_VDEV_HANDLE BIT(3)
> +
> +struct vdpa_opts {
> + uint64_t present; /* flags of present items */
> + const char *mdev_bus_name;
> + const char *mdev_name;
> + const char *vdev_name;
> + unsigned int device_id;
> +};
> +
> +struct vdpa {
> + struct vdpa_socket nlg;
> + struct vdpa_opts opts;
> + bool json_output;
> +};
> +
> +static void indent_inc(void)
> +{
> + if (g_indent_level + INDENT_STR_STEP > INDENT_STR_MAXLEN)
> + return;
> + g_indent_level += INDENT_STR_STEP;
> + memset(g_indent_str, ' ', sizeof(g_indent_str));
> + g_indent_str[g_indent_level] = '\0';
> +}
> +
> +static void 

RE: [PATCH v5 12/16] asm-generic/hyperv: update hv_interrupt_entry

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> We will soon use the same structure to handle IO-APIC interrupts as
> well. Introduce an enum to identify the source and a data structure for
> IO-APIC RTE.
> 
> While at it, update pci-hyperv.c to use the enum.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu 
> Acked-by: Rob Herring 
> ---
>  drivers/pci/controller/pci-hyperv.c |  2 +-
>  include/asm-generic/hyperv-tlfs.h   | 36 +++--
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c 
> b/drivers/pci/controller/pci-hyperv.c
> index 6db8d96a78eb..87aa62ee0368 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1216,7 +1216,7 @@ static void hv_irq_unmask(struct irq_data *data)
>   params = >retarget_msi_interrupt_params;
>   memset(params, 0, sizeof(*params));
>   params->partition_id = HV_PARTITION_ID_SELF;
> - params->int_entry.source = 1; /* MSI(-X) */
> + params->int_entry.source = HV_INTERRUPT_SOURCE_MSI;
>   hv_set_msi_entry_from_desc(>int_entry.msi_entry, msi_desc);
>   params->device_id = (hbus->hdev->dev_instance.b[5] << 24) |
>  (hbus->hdev->dev_instance.b[4] << 16) |
> diff --git a/include/asm-generic/hyperv-tlfs.h 
> b/include/asm-generic/hyperv-tlfs.h
> index 7e103be42799..8423bf53c237 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -480,6 +480,11 @@ struct hv_create_vp {
>   u64 flags;
>  } __packed;
> 
> +enum hv_interrupt_source {
> + HV_INTERRUPT_SOURCE_MSI = 1, /* MSI and MSI-X */
> + HV_INTERRUPT_SOURCE_IOAPIC,
> +};
> +
>  union hv_msi_address_register {
>   u32 as_uint32;
>   struct {
> @@ -513,10 +518,37 @@ union hv_msi_entry {
>   } __packed;
>  };
> 
> +union hv_ioapic_rte {
> + u64 as_uint64;
> +
> + struct {
> + u32 vector:8;
> + u32 delivery_mode:3;
> + u32 destination_mode:1;
> + u32 delivery_status:1;
> + u32 interrupt_polarity:1;
> + u32 remote_irr:1;
> + u32 trigger_mode:1;
> + u32 interrupt_mask:1;
> + u32 reserved1:15;
> +
> + u32 reserved2:24;
> + u32 destination_id:8;
> + };
> +
> + struct {
> + u32 low_uint32;
> + u32 high_uint32;
> + };
> +} __packed;
> +
>  struct hv_interrupt_entry {
> - u32 source; /* 1 for MSI(-X) */
> + u32 source;
>   u32 reserved1;
> - union hv_msi_entry msi_entry;
> + union {
> + union hv_msi_entry msi_entry;
> + union hv_ioapic_rte ioapic_rte;
> + };
>  } __packed;
> 
>  /*
> --
> 2.20.1

Reviewed-by: Michael Kelley 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 09/16] x86/hyperv: provide a bunch of helper functions

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> They are used to deposit pages into Microsoft Hypervisor and bring up
> logical and virtual processors.
> 
> Signed-off-by: Lillian Grassin-Drake 
> Signed-off-by: Sunil Muthuswamy 
> Signed-off-by: Nuno Das Neves 
> Co-Developed-by: Lillian Grassin-Drake 
> Co-Developed-by: Sunil Muthuswamy 
> Co-Developed-by: Nuno Das Neves 
> Signed-off-by: Wei Liu 
> ---
> v4: Fix compilation issue when CONFIG_ACPI_NUMA is not set.
> 
> v3:
> 1. Add __packed to structures.
> 2. Drop unnecessary exports.
> 
> v2:
> 1. Adapt to hypervisor side changes
> 2. Address Vitaly's comments
> ---
>  arch/x86/hyperv/Makefile  |   2 +-
>  arch/x86/hyperv/hv_proc.c | 225 ++
>  arch/x86/include/asm/mshyperv.h   |   4 +
>  include/asm-generic/hyperv-tlfs.h |  67 +
>  4 files changed, 297 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/hyperv/hv_proc.c
> 
> diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile
> index 89b1f74d3225..565358020921 100644
> --- a/arch/x86/hyperv/Makefile
> +++ b/arch/x86/hyperv/Makefile
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-y:= hv_init.o mmu.o nested.o
> -obj-$(CONFIG_X86_64) += hv_apic.o
> +obj-$(CONFIG_X86_64) += hv_apic.o hv_proc.o
> 
>  ifdef CONFIG_X86_64
>  obj-$(CONFIG_PARAVIRT_SPINLOCKS) += hv_spinlock.o
> diff --git a/arch/x86/hyperv/hv_proc.c b/arch/x86/hyperv/hv_proc.c
> new file mode 100644
> index ..706097160e2f
> --- /dev/null
> +++ b/arch/x86/hyperv/hv_proc.c
> @@ -0,0 +1,225 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#define HV_DEPOSIT_MAX_ORDER (8)
> +#define HV_DEPOSIT_MAX (1 << HV_DEPOSIT_MAX_ORDER)

Is there any reason to not let the maximum be 511, which is
how many entries will fit on the hypercall input page?  The
max could be define in terms of HY_HYP_PAGE_SIZE so that
the logical dependency is fully expressed.  

> +
> +/*
> + * Deposits exact number of pages
> + * Must be called with interrupts enabled
> + * Max 256 pages
> + */
> +int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> +{
> + struct page **pages;
> + int *counts;
> + int num_allocations;
> + int i, j, page_count;
> + int order;
> + int desired_order;
> + u16 status;
> + int ret;
> + u64 base_pfn;
> + struct hv_deposit_memory *input_page;
> + unsigned long flags;
> +
> + if (num_pages > HV_DEPOSIT_MAX)
> + return -E2BIG;
> + if (!num_pages)
> + return 0;
> +
> + /* One buffer for page pointers and counts */
> + pages = page_address(alloc_page(GFP_KERNEL));
> + if (!pages)

Does the above check work?  If alloc_pages() returns NULL, it looks like
page_address() might fault.

> + return -ENOMEM;
> +
> + counts = kcalloc(HV_DEPOSIT_MAX, sizeof(int), GFP_KERNEL);
> + if (!counts) {
> + free_page((unsigned long)pages);
> + return -ENOMEM;
> + }
> +
> + /* Allocate all the pages before disabling interrupts */
> + num_allocations = 0;
> + i = 0;
> + order = HV_DEPOSIT_MAX_ORDER;
> +
> + while (num_pages) {
> + /* Find highest order we can actually allocate */
> + desired_order = 31 - __builtin_clz(num_pages);
> + order = min(desired_order, order);

The above seems redundant since request sizes larger than the
max have already been rejected.

> + do {
> + pages[i] = alloc_pages_node(node, GFP_KERNEL, order);
> + if (!pages[i]) {
> + if (!order) {
> + ret = -ENOMEM;
> + goto err_free_allocations;
> + }
> + --order;
> + }
> + } while (!pages[i]);

The duplicative test of !pages[i] is somewhat annoying.  How about
this:

while{!pages[i] = alloc_pages_node(node, GFP_KERNEL, order) {
if (!order) {
ret = -ENOMEM;
goto err_free_allocations;
}
--order;
}

or if you don't like doing an assignment in the while test:

while(1) {
pages[i] = alloc_pages_node(node, GFP_KERNEL, order);
if (page[i])
break;
if (!order) {
ret = -ENOMEM;
goto err_free_allocations;
}
--order;
}

> +
> + 

RE: [PATCH v5 10/16] x86/hyperv: implement and use hv_smp_prepare_cpus

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> Microsoft Hypervisor requires the root partition to make a few
> hypercalls to setup application processors before they can be used.
> 
> Signed-off-by: Lillian Grassin-Drake 
> Signed-off-by: Sunil Muthuswamy 
> Co-Developed-by: Lillian Grassin-Drake 
> Co-Developed-by: Sunil Muthuswamy 
> Signed-off-by: Wei Liu 
> ---
> CPU hotplug and unplug is not yet supported in this setup, so those
> paths remain untouched.
> 
> v3: Always call native SMP preparation function.
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 29 +
>  1 file changed, 29 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index c376d191a260..13d3b6dd21a3 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  /* Is Linux running as the root partition? */
>  bool hv_root_partition;
> @@ -212,6 +213,32 @@ static void __init hv_smp_prepare_boot_cpu(void)
>   hv_init_spinlocks();
>  #endif
>  }
> +
> +static void __init hv_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +#ifdef CONFIG_X86_64
> + int i;
> + int ret;
> +#endif
> +
> + native_smp_prepare_cpus(max_cpus);
> +
> +#ifdef CONFIG_X86_64
> + for_each_present_cpu(i) {
> + if (i == 0)
> + continue;
> + ret = hv_call_add_logical_proc(numa_cpu_node(i), i, 
> cpu_physical_id(i));
> + BUG_ON(ret);
> + }
> +
> + for_each_present_cpu(i) {
> + if (i == 0)
> + continue;
> + ret = hv_call_create_vp(numa_cpu_node(i), 
> hv_current_partition_id, i, i);
> + BUG_ON(ret);
> + }
> +#endif
> +}
>  #endif
> 
>  static void __init ms_hyperv_init_platform(void)
> @@ -368,6 +395,8 @@ static void __init ms_hyperv_init_platform(void)
> 
>  # ifdef CONFIG_SMP
>   smp_ops.smp_prepare_boot_cpu = hv_smp_prepare_boot_cpu;
> + if (hv_root_partition)
> + smp_ops.smp_prepare_cpus = hv_smp_prepare_cpus;
>  # endif
> 
>   /*
> --
> 2.20.1

Reviewed-by: Michael Kelley 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 11/16] asm-generic/hyperv: update hv_msi_entry

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> We will soon need to access fields inside the MSI address and MSI data
> fields. Introduce hv_msi_address_register and hv_msi_data_register.
> 
> Fix up one user of hv_msi_entry in mshyperv.h.
> 
> No functional change expected.
> 
> Signed-off-by: Wei Liu 
> ---
>  arch/x86/include/asm/mshyperv.h   |  4 ++--
>  include/asm-generic/hyperv-tlfs.h | 28 ++--
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 4e590a167160..cbee72550a12 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -257,8 +257,8 @@ static inline void hv_apic_init(void) {}
>  static inline void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
> struct msi_desc *msi_desc)
>  {
> - msi_entry->address = msi_desc->msg.address_lo;
> - msi_entry->data = msi_desc->msg.data;
> + msi_entry->address.as_uint32 = msi_desc->msg.address_lo;
> + msi_entry->data.as_uint32 = msi_desc->msg.data;
>  }
> 
>  #else /* CONFIG_HYPERV */
> diff --git a/include/asm-generic/hyperv-tlfs.h 
> b/include/asm-generic/hyperv-tlfs.h
> index ec53570102f0..7e103be42799 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -480,12 +480,36 @@ struct hv_create_vp {
>   u64 flags;
>  } __packed;
> 
> +union hv_msi_address_register {
> + u32 as_uint32;
> + struct {
> + u32 reserved1:2;
> + u32 destination_mode:1;
> + u32 redirection_hint:1;
> + u32 reserved2:8;
> + u32 destination_id:8;
> + u32 msi_base:12;
> + };
> +} __packed;
> +
> +union hv_msi_data_register {
> + u32 as_uint32;
> + struct {
> + u32 vector:8;
> + u32 delivery_mode:3;
> + u32 reserved1:3;
> + u32 level_assert:1;
> + u32 trigger_mode:1;
> + u32 reserved2:16;
> + };
> +} __packed;
> +
>  /* HvRetargetDeviceInterrupt hypercall */
>  union hv_msi_entry {
>   u64 as_uint64;
>   struct {
> - u32 address;
> - u32 data;
> + union hv_msi_address_register address;
> + union hv_msi_data_register data;
>   } __packed;
>  };
> 
> --
> 2.20.1

Reviewed-by: Michael Kelley 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 06/16] x86/hyperv: allocate output arg pages if required

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> When Linux runs as the root partition, it will need to make hypercalls
> which return data from the hypervisor.
> 
> Allocate pages for storing results when Linux runs as the root
> partition.
> 
> Signed-off-by: Lillian Grassin-Drake 
> Co-Developed-by: Lillian Grassin-Drake 
> Signed-off-by: Wei Liu 
> ---
> v3: Fix hv_cpu_die to use free_pages.
> v2: Address Vitaly's comments
> ---
>  arch/x86/hyperv/hv_init.c   | 35 -
>  arch/x86/include/asm/mshyperv.h |  1 +
>  2 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index e04d90af4c27..6f4cb40e53fe 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -41,6 +41,9 @@ EXPORT_SYMBOL_GPL(hv_vp_assist_page);
>  void  __percpu **hyperv_pcpu_input_arg;
>  EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
> 
> +void  __percpu **hyperv_pcpu_output_arg;
> +EXPORT_SYMBOL_GPL(hyperv_pcpu_output_arg);
> +
>  u32 hv_max_vp_index;
>  EXPORT_SYMBOL_GPL(hv_max_vp_index);
> 
> @@ -73,12 +76,19 @@ static int hv_cpu_init(unsigned int cpu)
>   void **input_arg;
>   struct page *pg;
> 
> - input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
>   /* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
> - pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
> + pg = alloc_pages(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL, 
> hv_root_partition ?
> 1 : 0);
>   if (unlikely(!pg))
>   return -ENOMEM;
> +
> + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
>   *input_arg = page_address(pg);
> + if (hv_root_partition) {
> + void **output_arg;
> +
> + output_arg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
> + *output_arg = page_address(pg + 1);
> + }
> 
>   hv_get_vp_index(msr_vp_index);
> 
> @@ -205,14 +215,23 @@ static int hv_cpu_die(unsigned int cpu)
>   unsigned int new_cpu;
>   unsigned long flags;
>   void **input_arg;
> - void *input_pg = NULL;
> + void *pg;
> 
>   local_irq_save(flags);
>   input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> - input_pg = *input_arg;
> + pg = *input_arg;
>   *input_arg = NULL;
> +
> + if (hv_root_partition) {
> + void **output_arg;
> +
> + output_arg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
> + *output_arg = NULL;
> + }
> +
>   local_irq_restore(flags);
> - free_page((unsigned long)input_pg);
> +
> + free_pages((unsigned long)pg, hv_root_partition ? 1 : 0);
> 
>   if (hv_vp_assist_page && hv_vp_assist_page[cpu])
>   wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> @@ -346,6 +365,12 @@ void __init hyperv_init(void)
> 
>   BUG_ON(hyperv_pcpu_input_arg == NULL);
> 
> + /* Allocate the per-CPU state for output arg for root */
> + if (hv_root_partition) {
> + hyperv_pcpu_output_arg = alloc_percpu(void *);
> + BUG_ON(hyperv_pcpu_output_arg == NULL);
> + }
> +
>   /* Allocate percpu VP index */
>   hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
>   GFP_KERNEL);
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index ac2b0d110f03..62d9390f1ddf 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -76,6 +76,7 @@ static inline void hv_disable_stimer0_percpu_irq(int irq) {}
>  #if IS_ENABLED(CONFIG_HYPERV)
>  extern void *hv_hypercall_pg;
>  extern void  __percpu  **hyperv_pcpu_input_arg;
> +extern void  __percpu  **hyperv_pcpu_output_arg;
> 
>  static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
>  {
> --
> 2.20.1

I think this all works OK.  But a meta question:  Do we need a separate
per-cpu output argument page?  From the Hyper-V hypercall standpoint, I
don't think input and output args need to be in separate pages.  They both
just need to not cross a page boundary.  As long as we don't have a hypercall
where the sum of the sizes of the input and output args exceeds a page,
we could just have a single page, and split it up in any manner that works
for the particular hypercall.

Thoughts?

Michael


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 14/16] asm-generic/hyperv: import data structures for mapping device interrupts

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> Signed-off-by: Sunil Muthuswamy 
> Co-Developed-by: Sunil Muthuswamy 
> Signed-off-by: Wei Liu 
> ---
>  arch/x86/include/asm/hyperv-tlfs.h | 13 +++
>  include/asm-generic/hyperv-tlfs.h  | 36 ++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h 
> b/arch/x86/include/asm/hyperv-tlfs.h
> index 204010350604..ab7d6cde548d 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -533,6 +533,19 @@ struct hv_partition_assist_pg {
>   u32 tlb_lock_count;
>  };
> 
> +enum hv_interrupt_type {
> + HV_X64_INTERRUPT_TYPE_FIXED = 0x,
> + HV_X64_INTERRUPT_TYPE_LOWESTPRIORITY= 0x0001,
> + HV_X64_INTERRUPT_TYPE_SMI   = 0x0002,
> + HV_X64_INTERRUPT_TYPE_REMOTEREAD= 0x0003,
> + HV_X64_INTERRUPT_TYPE_NMI   = 0x0004,
> + HV_X64_INTERRUPT_TYPE_INIT  = 0x0005,
> + HV_X64_INTERRUPT_TYPE_SIPI  = 0x0006,
> + HV_X64_INTERRUPT_TYPE_EXTINT= 0x0007,
> + HV_X64_INTERRUPT_TYPE_LOCALINT0 = 0x0008,
> + HV_X64_INTERRUPT_TYPE_LOCALINT1 = 0x0009,
> + HV_X64_INTERRUPT_TYPE_MAXIMUM   = 0x000A,
> +};
> 
>  #include 
> 
> diff --git a/include/asm-generic/hyperv-tlfs.h 
> b/include/asm-generic/hyperv-tlfs.h
> index 42ff1326c6bd..07efe0131fe3 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -152,6 +152,8 @@ struct ms_hyperv_tsc_page {
>  #define HVCALL_RETRIEVE_DEBUG_DATA   0x006a
>  #define HVCALL_RESET_DEBUG_SESSION   0x006b
>  #define HVCALL_ADD_LOGICAL_PROCESSOR 0x0076
> +#define HVCALL_MAP_DEVICE_INTERRUPT  0x007c
> +#define HVCALL_UNMAP_DEVICE_INTERRUPT0x007d
>  #define HVCALL_RETARGET_INTERRUPT0x007e
>  #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
>  #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
> @@ -702,4 +704,38 @@ union hv_device_id {
>   } acpi;
>  } __packed;
> 
> +enum hv_interrupt_trigger_mode {
> + HV_INTERRUPT_TRIGGER_MODE_EDGE = 0,
> + HV_INTERRUPT_TRIGGER_MODE_LEVEL = 1,
> +};
> +
> +struct hv_device_interrupt_descriptor {
> + u32 interrupt_type;
> + u32 trigger_mode;
> + u32 vector_count;
> + u32 reserved;
> + struct hv_device_interrupt_target target;
> +} __packed;
> +
> +struct hv_input_map_device_interrupt {
> + u64 partition_id;
> + u64 device_id;
> + u64 flags;
> + struct hv_interrupt_entry logical_interrupt_entry;
> + struct hv_device_interrupt_descriptor interrupt_descriptor;
> +} __packed;
> +
> +struct hv_output_map_device_interrupt {
> + struct hv_interrupt_entry interrupt_entry;
> +} __packed;
> +
> +struct hv_input_unmap_device_interrupt {
> + u64 partition_id;
> + u64 device_id;
> + struct hv_interrupt_entry interrupt_entry;
> +} __packed;
> +
> +#define HV_SOURCE_SHADOW_NONE   0x0
> +#define HV_SOURCE_SHADOW_BRIDGE_BUS_RANGE   0x1
> +
>  #endif
> --
> 2.20.1

Reviewed-by: Michael Kelley 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 13/16] asm-generic/hyperv: introduce hv_device_id and auxiliary structures

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> We will need to identify the device we want Microsoft Hypervisor to
> manipulate.  Introduce the data structures for that purpose.
> 
> They will be used in a later patch.
> 
> Signed-off-by: Sunil Muthuswamy 
> Co-Developed-by: Sunil Muthuswamy 
> Signed-off-by: Wei Liu 
> ---
>  include/asm-generic/hyperv-tlfs.h | 79 +++
>  1 file changed, 79 insertions(+)
> 
> diff --git a/include/asm-generic/hyperv-tlfs.h 
> b/include/asm-generic/hyperv-tlfs.h
> index 8423bf53c237..42ff1326c6bd 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -623,4 +623,83 @@ struct hv_set_vp_registers_input {
>   } element[];
>  } __packed;
> 
> +enum hv_device_type {
> + HV_DEVICE_TYPE_LOGICAL = 0,
> + HV_DEVICE_TYPE_PCI = 1,
> + HV_DEVICE_TYPE_IOAPIC = 2,
> + HV_DEVICE_TYPE_ACPI = 3,
> +};
> +
> +typedef u16 hv_pci_rid;
> +typedef u16 hv_pci_segment;
> +typedef u64 hv_logical_device_id;
> +union hv_pci_bdf {
> + u16 as_uint16;
> +
> + struct {
> + u8 function:3;
> + u8 device:5;
> + u8 bus;
> + };
> +} __packed;
> +
> +union hv_pci_bus_range {
> + u16 as_uint16;
> +
> + struct {
> + u8 subordinate_bus;
> + u8 secondary_bus;
> + };
> +} __packed;
> +
> +union hv_device_id {
> + u64 as_uint64;
> +
> + struct {
> + u64 :62;
> + u64 device_type:2;
> + };

Are the above 4 lines extraneous junk? 
If not, a comment would be helpful.  And we
would normally label the 62 bit field as 
"reserved0" or something similar.

> +
> + /* HV_DEVICE_TYPE_LOGICAL */
> + struct {
> + u64 id:62;
> + u64 device_type:2;
> + } logical;
> +
> + /* HV_DEVICE_TYPE_PCI */
> + struct {
> + union {
> + hv_pci_rid rid;
> + union hv_pci_bdf bdf;
> + };
> +
> + hv_pci_segment segment;
> + union hv_pci_bus_range shadow_bus_range;
> +
> + u16 phantom_function_bits:2;
> + u16 source_shadow:1;
> +
> + u16 rsvdz0:11;
> + u16 device_type:2;
> + } pci;
> +
> + /* HV_DEVICE_TYPE_IOAPIC */
> + struct {
> + u8 ioapic_id;
> + u8 rsvdz0;
> + u16 rsvdz1;
> + u16 rsvdz2;
> +
> + u16 rsvdz3:14;
> + u16 device_type:2;
> + } ioapic;
> +
> + /* HV_DEVICE_TYPE_ACPI */
> + struct {
> + u32 input_mapping_base;
> + u32 input_mapping_count:30;
> + u32 device_type:2;
> + } acpi;
> +} __packed;
> +
>  #endif
> --
> 2.20.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 08/16] x86/hyperv: handling hypercall page setup for root

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> When Linux is running as the root partition, the hypercall page will
> have already been setup by Hyper-V. Copy the content over to the
> allocated page.
> 
> Add checks to hv_suspend & co to bail early because they are not
> supported in this setup yet.
> 
> Signed-off-by: Lillian Grassin-Drake 
> Signed-off-by: Sunil Muthuswamy 
> Signed-off-by: Nuno Das Neves 
> Co-Developed-by: Lillian Grassin-Drake 
> Co-Developed-by: Sunil Muthuswamy 
> Co-Developed-by: Nuno Das Neves 
> Signed-off-by: Wei Liu 
> ---
> v3:
> 1. Use HV_HYP_PAGE_SIZE.
> 2. Add checks to hv_suspend & co.
> ---
>  arch/x86/hyperv/hv_init.c | 37 ++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index fc9941bd8653..ad8e77859b32 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  u64 hv_current_partition_id = ~0ull;
>  EXPORT_SYMBOL_GPL(hv_current_partition_id);
> @@ -283,6 +284,9 @@ static int hv_suspend(void)
>   union hv_x64_msr_hypercall_contents hypercall_msr;
>   int ret;
> 
> + if (hv_root_partition)
> + return -EPERM;
> +
>   /*
>* Reset the hypercall page as it is going to be invalidated
>* accross hibernation. Setting hv_hypercall_pg to NULL ensures
> @@ -433,8 +437,35 @@ void __init hyperv_init(void)
> 
>   rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
>   hypercall_msr.enable = 1;
> - hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
> - wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +
> + if (hv_root_partition) {
> + struct page *pg;
> + void *src, *dst;
> +
> + /*
> +  * For the root partition, the hypervisor will set up its
> +  * hypercall page. The hypervisor guarantees it will not show
> +  * up in the root's address space. The root can't change the
> +  * location of the hypercall page.
> +  *
> +  * Order is important here. We must enable the hypercall page
> +  * so it is populated with code, then copy the code to an
> +  * executable page.
> +  */
> + wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +
> + pg = vmalloc_to_page(hv_hypercall_pg);
> + dst = kmap(pg);
> + src = memremap(hypercall_msr.guest_physical_address << 
> PAGE_SHIFT,
> PAGE_SIZE,
> + MEMREMAP_WB);
> + BUG_ON(!(src && dst));
> + memcpy(dst, src, HV_HYP_PAGE_SIZE);
> + memunmap(src);
> + kunmap(pg);
> + } else {
> + hypercall_msr.guest_physical_address = 
> vmalloc_to_pfn(hv_hypercall_pg);
> + wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> + }
> 
>   /*
>* Ignore any errors in setting up stimer clockevents
> @@ -577,6 +608,6 @@ EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
> 
>  bool hv_is_hibernation_supported(void)
>  {
> - return acpi_sleep_state_supported(ACPI_STATE_S4);
> + return !hv_root_partition && acpi_sleep_state_supported(ACPI_STATE_S4);
>  }
>  EXPORT_SYMBOL_GPL(hv_is_hibernation_supported);
> --
> 2.20.1

Reviewed-by: Michael Kelley 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 07/16] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> We will need the partition ID for executing some hypercalls later.
> 
> Signed-off-by: Lillian Grassin-Drake 
> Co-Developed-by: Sunil Muthuswamy 
> Signed-off-by: Wei Liu 
> ---
> v3:
> 1. Make hv_get_partition_id static.
> 2. Change code structure a bit.
> ---
>  arch/x86/hyperv/hv_init.c | 27 +++
>  arch/x86/include/asm/mshyperv.h   |  2 ++
>  include/asm-generic/hyperv-tlfs.h |  6 ++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 6f4cb40e53fe..fc9941bd8653 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -26,6 +26,9 @@
>  #include 
>  #include 
> 
> +u64 hv_current_partition_id = ~0ull;
> +EXPORT_SYMBOL_GPL(hv_current_partition_id);
> +
>  void *hv_hypercall_pg;
>  EXPORT_SYMBOL_GPL(hv_hypercall_pg);
> 
> @@ -331,6 +334,25 @@ static struct syscore_ops hv_syscore_ops = {
>   .resume = hv_resume,
>  };
> 
> +static void __init hv_get_partition_id(void)
> +{
> + struct hv_get_partition_id *output_page;
> + u16 status;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
> + status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
> + HV_HYPERCALL_RESULT_MASK;
> + if (status != HV_STATUS_SUCCESS) {

Across the Hyper-V code in Linux, the way we check the hypercall result
is very inconsistent.  IMHO, the and'ing of hv_do_hypercall() with 
HV_HYPERCALL_RESULT_MASK so that status can be a u16 is stylistically
a bit unusual.

I'd like to see the hypercall result being stored into a u64 local variable.
Then the subsequent test for the status should 'and' the u64 with
HV_HYPERCALL_RESULT_MASK to determine the result code.
I've made a note to go fix the places that aren't doing it that way.

> + /* No point in proceeding if this failed */
> + pr_err("Failed to get partition ID: %d\n", status);
> + BUG();
> + }
> + hv_current_partition_id = output_page->partition_id;
> + local_irq_restore(flags);
> +}
> +
>  /*
>   * This function is to be invoked early in the boot sequence after the
>   * hypervisor has been detected.
> @@ -426,6 +448,11 @@ void __init hyperv_init(void)
> 
>   register_syscore_ops(_syscore_ops);
> 
> + if (cpuid_ebx(HYPERV_CPUID_FEATURES) & HV_ACCESS_PARTITION_ID)
> + hv_get_partition_id();

Another place where the EBX value saved into the ms_hyperv structure
could be used.

> +
> + BUG_ON(hv_root_partition && hv_current_partition_id == ~0ull);
> +
>   return;
> 
>  remove_cpuhp_state:
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 62d9390f1ddf..67f5d35a73d3 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -78,6 +78,8 @@ extern void *hv_hypercall_pg;
>  extern void  __percpu  **hyperv_pcpu_input_arg;
>  extern void  __percpu  **hyperv_pcpu_output_arg;
> 
> +extern u64 hv_current_partition_id;
> +
>  static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
>  {
>   u64 input_address = input ? virt_to_phys(input) : 0;
> diff --git a/include/asm-generic/hyperv-tlfs.h 
> b/include/asm-generic/hyperv-tlfs.h
> index e6903589a82a..87b1a79b19eb 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -141,6 +141,7 @@ struct ms_hyperv_tsc_page {
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX0x0013
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX 0x0014
>  #define HVCALL_SEND_IPI_EX   0x0015
> +#define HVCALL_GET_PARTITION_ID  0x0046
>  #define HVCALL_GET_VP_REGISTERS  0x0050
>  #define HVCALL_SET_VP_REGISTERS  0x0051
>  #define HVCALL_POST_MESSAGE  0x005c
> @@ -407,6 +408,11 @@ struct hv_tlb_flush_ex {
>   u64 gva_list[];
>  } __packed;
> 
> +/* HvGetPartitionId hypercall (output only) */
> +struct hv_get_partition_id {
> + u64 partition_id;
> +} __packed;
> +
>  /* HvRetargetDeviceInterrupt hypercall */
>  union hv_msi_entry {
>   u64 as_uint64;
> --
> 2.20.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 01/16] asm-generic/hyperv: change HV_CPU_POWER_MANAGEMENT to HV_CPU_MANAGEMENT

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> This makes the name match Hyper-V TLFS.
> 
> Signed-off-by: Wei Liu 
> Reviewed-by: Vitaly Kuznetsov 
> ---
>  include/asm-generic/hyperv-tlfs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/asm-generic/hyperv-tlfs.h 
> b/include/asm-generic/hyperv-tlfs.h
> index e73a11850055..e6903589a82a 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -88,7 +88,7 @@
>  #define HV_CONNECT_PORT  BIT(7)
>  #define HV_ACCESS_STATS  BIT(8)
>  #define HV_DEBUGGING BIT(11)
> -#define HV_CPU_POWER_MANAGEMENT  BIT(12)
> +#define HV_CPU_MANAGEMENTBIT(12)
> 
> 
>  /*
> --
> 2.20.1

Reviewed-by: Michael Kelley 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 05/16] clocksource/hyperv: use MSR-based access if running as root

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> When Linux runs as the root partition, the setup required for TSC page
> is different. Luckily Linux also has access to the MSR based
> clocksource. We can just disable the TSC page clocksource if Linux is
> the root partition.
> 
> Signed-off-by: Wei Liu 
> Acked-by: Daniel Lezcano 
> ---
>  drivers/clocksource/hyperv_timer.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/clocksource/hyperv_timer.c 
> b/drivers/clocksource/hyperv_timer.c
> index ba04cb381cd3..269a691bd2c4 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -426,6 +426,9 @@ static bool __init hv_init_tsc_clocksource(void)
>   if (!(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE))
>   return false;
> 
> + if (hv_root_partition)
> + return false;
> +
>   hv_read_reference_counter = read_hv_clock_tsc;
>   phys_addr = virt_to_phys(hv_get_tsc_page());
> 
> --
> 2.20.1

Reviewed-by: Michael Kelley 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 04/16] iommu/hyperv: don't setup IRQ remapping when running as root

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> The IOMMU code needs more work. We're sure for now the IRQ remapping
> hooks are not applicable when Linux is the root partition.
> 
> Signed-off-by: Wei Liu 
> Acked-by: Joerg Roedel 
> Reviewed-by: Vitaly Kuznetsov 
> ---
>  drivers/iommu/hyperv-iommu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> index 1d21a0b5f724..b7db6024e65c 100644
> --- a/drivers/iommu/hyperv-iommu.c
> +++ b/drivers/iommu/hyperv-iommu.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include "irq_remapping.h"
> 
> @@ -122,7 +123,7 @@ static int __init hyperv_prepare_irq_remapping(void)
> 
>   if (!hypervisor_is_type(X86_HYPER_MS_HYPERV) ||
>   x86_init.hyper.msi_ext_dest_id() ||
> - !x2apic_supported())
> + !x2apic_supported() || hv_root_partition)
>   return -ENODEV;
> 
>   fn = irq_domain_alloc_named_id_fwnode("HYPERV-IR", 0);
> --
> 2.20.1

Reviewed-by: Michael Kelley 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 03/16] Drivers: hv: vmbus: skip VMBus initialization if Linux is root

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> There is no VMBus and the other infrastructures initialized in
> hv_acpi_init when Linux is running as the root partition.
> 
> Signed-off-by: Wei Liu 
> ---
> v3: Return 0 instead of -ENODEV.
> ---
>  drivers/hv/vmbus_drv.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 502f8cd95f6d..ee27b3670a51 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -2620,6 +2620,9 @@ static int __init hv_acpi_init(void)
>   if (!hv_is_hyperv_initialized())
>   return -ENODEV;
> 
> + if (hv_root_partition)
> + return 0;
> +
>   init_completion(_event);
> 
>   /*
> --
> 2.20.1

Reviewed-by: Michael Kelley 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH v5 02/16] x86/hyperv: detect if Linux is the root partition

[Michael Kelley via Virtualization]

From: Wei Liu  Sent: Wednesday, January 20, 2021 4:01 AM
> 
> For now we can use the privilege flag to check. Stash the value to be
> used later.
> 
> Put in a bunch of defines for future use when we want to have more
> fine-grained detection.
> 
> Signed-off-by: Wei Liu 
> ---
> v3: move hv_root_partition to mshyperv.c
> ---
>  arch/x86/include/asm/hyperv-tlfs.h | 10 ++
>  arch/x86/include/asm/mshyperv.h|  2 ++
>  arch/x86/kernel/cpu/mshyperv.c | 20 
>  3 files changed, 32 insertions(+)
> 
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h 
> b/arch/x86/include/asm/hyperv-tlfs.h
> index 6bf42aed387e..204010350604 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -21,6 +21,7 @@
>  #define HYPERV_CPUID_FEATURES0x4003
>  #define HYPERV_CPUID_ENLIGHTMENT_INFO0x4004
>  #define HYPERV_CPUID_IMPLEMENT_LIMITS0x4005
> +#define HYPERV_CPUID_CPU_MANAGEMENT_FEATURES 0x4007
>  #define HYPERV_CPUID_NESTED_FEATURES 0x400A
> 
>  #define HYPERV_CPUID_VIRT_STACK_INTERFACE0x4081
> @@ -110,6 +111,15 @@
>  /* Recommend using enlightened VMCS */
>  #define HV_X64_ENLIGHTENED_VMCS_RECOMMENDED  BIT(14)
> 
> +/*
> + * CPU management features identification.
> + * These are HYPERV_CPUID_CPU_MANAGEMENT_FEATURES.EAX bits.
> + */
> +#define HV_X64_START_LOGICAL_PROCESSOR   BIT(0)
> +#define HV_X64_CREATE_ROOT_VIRTUAL_PROCESSOR BIT(1)
> +#define HV_X64_PERFORMANCE_COUNTER_SYNC  BIT(2)
> +#define HV_X64_RESERVED_IDENTITY_BIT BIT(31)
> +

I wonder if these bit definitions should go in the asm-generic part of
hyperv-tlfs.h instead of the X64 specific part.  They look very architecture
neutral (in which case the X64 should be dropped from the name
as well).  Of course, they can be moved later when/if we get to that point
and have a firmer understanding of what is and isn't arch neutral.

>  /*
>   * Virtual processor will never share a physical core with another virtual
>   * processor, except for virtual processors that are reported as sibling SMT
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index ffc289992d1b..ac2b0d110f03 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -237,6 +237,8 @@ int hyperv_fill_flush_guest_mapping_list(
>   struct hv_guest_mapping_flush_list *flush,
>   u64 start_gfn, u64 end_gfn);
> 
> +extern bool hv_root_partition;
> +
>  #ifdef CONFIG_X86_64
>  void hv_apic_init(void);
>  void __init hv_init_spinlocks(void);
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index f628e3dc150f..c376d191a260 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -32,6 +32,10 @@
>  #include 
>  #include 
> 
> +/* Is Linux running as the root partition? */
> +bool hv_root_partition;
> +EXPORT_SYMBOL_GPL(hv_root_partition);
> +
>  struct ms_hyperv_info ms_hyperv;
>  EXPORT_SYMBOL_GPL(ms_hyperv);
> 
> @@ -237,6 +241,22 @@ static void __init ms_hyperv_init_platform(void)
>   pr_debug("Hyper-V: max %u virtual processors, %u logical processors\n",
>ms_hyperv.max_vp_index, ms_hyperv.max_lp_index);
> 
> + /*
> +  * Check CPU management privilege.
> +  *
> +  * To mirror what Windows does we should extract CPU management
> +  * features and use the ReservedIdentityBit to detect if Linux is the
> +  * root partition. But that requires negotiating CPU management
> +  * interface (a process to be finalized).
> +  *
> +  * For now, use the privilege flag as the indicator for running as
> +  * root.
> +  */
> + if (cpuid_ebx(HYPERV_CPUID_FEATURES) & HV_CPU_MANAGEMENT) {

Should the EBX value be captured in the ms_hyperv structure with the
other similar values, and then used from there?

Michael

> + hv_root_partition = true;
> + pr_info("Hyper-V: running as root partition\n");
> + }
> +
>   /*
>* Extract host information.
>*/
> --
> 2.20.1


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 6/9] ALSA: virtio: PCM substream operators

[Guennadi Liakhovetski]

On Sun, 24 Jan 2021, Anton Yakovlev wrote:


Introduce the operators required for the operation of substreams.

Signed-off-by: Anton Yakovlev 
---
sound/virtio/Makefile |   3 +-
sound/virtio/virtio_pcm.c |   5 +-
sound/virtio/virtio_pcm.h |   2 +
sound/virtio/virtio_pcm_ops.c | 513 ++
4 files changed, 521 insertions(+), 2 deletions(-)
create mode 100644 sound/virtio/virtio_pcm_ops.c


[snip]


diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
new file mode 100644
index ..19882777fcd6
--- /dev/null
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -0,0 +1,513 @@


[snip]


+/**
+ * virtsnd_pcm_release() - Release the PCM substream on the device side.
+ * @substream: VirtIO substream.
+ *
+ * Context: Any context that permits to sleep.
+ * Return: 0 on success, -errno on failure.
+ */
+static inline bool virtsnd_pcm_released(struct virtio_pcm_substream *substream)
+{
+   /*
+* The spec states that upon receipt of the RELEASE command "the device
+* MUST complete all pending I/O messages for the specified stream ID".
+* Thus, we consider the absence of I/O messages in the queue as an
+* indication that the substream has been released.
+*/
+   return atomic_read(>msg_count) == 0;


Also here having it atomic doesn't really seem to help. This just means, 
that at some point of time it was == 0.



+}
+
+static int virtsnd_pcm_release(struct virtio_pcm_substream *substream)


kernel-doc missing


+{
+   struct virtio_snd *snd = substream->snd;
+   struct virtio_snd_msg *msg;
+   unsigned int js = msecs_to_jiffies(msg_timeout_ms);
+   int rc;
+
+   msg = virtsnd_pcm_ctl_msg_alloc(substream, VIRTIO_SND_R_PCM_RELEASE,
+   GFP_KERNEL);
+   if (IS_ERR(msg))
+   return PTR_ERR(msg);
+
+   rc = virtsnd_ctl_msg_send_sync(snd, msg);
+   if (rc)
+   return rc;
+
+   return wait_event_interruptible_timeout(substream->msg_empty,
+   virtsnd_pcm_released(substream),
+   js);
+}
+
+/**
+ * virtsnd_pcm_open() - Open the PCM substream.
+ * @substream: Kernel ALSA substream.
+ *
+ * Context: Any context.
+ * Return: 0 on success, -errno on failure.
+ */
+static int virtsnd_pcm_open(struct snd_pcm_substream *substream)
+{
+   struct virtio_pcm *pcm = snd_pcm_substream_chip(substream);
+   struct virtio_pcm_substream *ss = NULL;
+
+   if (pcm) {
+   switch (substream->stream) {
+   case SNDRV_PCM_STREAM_PLAYBACK:
+   case SNDRV_PCM_STREAM_CAPTURE: {
+   struct virtio_pcm_stream *stream =
+   >streams[substream->stream];
+
+   if (substream->number < stream->nsubstreams)


Can this condition ever be false?


+   ss = stream->substreams[substream->number];
+   break;
+   }
+   }
+   }
+
+   if (!ss)
+   return -EBADFD;
+
+   substream->runtime->hw = ss->hw;
+   substream->private_data = ss;
+
+   return 0;
+}
+
+/**
+ * virtsnd_pcm_close() - Close the PCM substream.
+ * @substream: Kernel ALSA substream.
+ *
+ * Context: Any context.
+ * Return: 0.
+ */
+static int virtsnd_pcm_close(struct snd_pcm_substream *substream)
+{
+   return 0;
+}
+
+/**
+ * virtsnd_pcm_hw_params() - Set the parameters of the PCM substream.
+ * @substream: Kernel ALSA substream.
+ * @hw_params: Hardware parameters (can be NULL).
+ *
+ * The function can be called both from the upper level (in this case,
+ * @hw_params is not NULL) or from the driver itself (in this case, @hw_params
+ * is NULL, and the parameter values are taken from the runtime structure).
+ *
+ * In all cases, the function:
+ *   1. checks the state of the virtqueue and, if necessary, tries to fix it,
+ *   2. sets the parameters on the device side,
+ *   3. allocates a hardware buffer and I/O messages.
+ *
+ * Context: Any context that permits to sleep.
+ * Return: 0 on success, -errno on failure.
+ */
+static int virtsnd_pcm_hw_params(struct snd_pcm_substream *substream,
+struct snd_pcm_hw_params *hw_params)
+{
+   struct snd_pcm_runtime *runtime = substream->runtime;
+   struct virtio_pcm_substream *ss = snd_pcm_substream_chip(substream);
+   struct virtio_device *vdev = ss->snd->vdev;
+   struct virtio_snd_msg *msg;
+   struct virtio_snd_pcm_set_params *request;
+   snd_pcm_format_t format;
+   unsigned int channels;
+   unsigned int rate;
+   unsigned int buffer_bytes;
+   unsigned int period_bytes;
+   unsigned int periods;
+   unsigned int i;
+   int vformat = -1;
+   int vrate = -1;
+   int rc;
+
+   /*
+* If we got here after ops->trigger() was called, the 

Re: [PATCH v2 5/9] ALSA: virtio: handling control and I/O messages for the PCM device

[Guennadi Liakhovetski]

On Sun, 24 Jan 2021, Anton Yakovlev wrote:


The driver implements a message-based transport for I/O substream
operations. Before the start of the substream, the hardware buffer is
sliced into I/O messages, the number of which is equal to the current
number of periods. The size of each message is equal to the current
size of one period.

I/O messages are organized in an ordered queue. The completion of the
I/O message indicates an elapsed period (the only exception is the end
of the stream for the capture substream). Upon completion, the message
is automatically re-added to the end of the queue.

Signed-off-by: Anton Yakovlev 
---
sound/virtio/Makefile |   3 +-
sound/virtio/virtio_card.c|  10 ++
sound/virtio/virtio_card.h|   9 +
sound/virtio/virtio_pcm.c |   3 +
sound/virtio/virtio_pcm.h |  31 
sound/virtio/virtio_pcm_msg.c | 325 ++
6 files changed, 380 insertions(+), 1 deletion(-)
create mode 100644 sound/virtio/virtio_pcm_msg.c

diff --git a/sound/virtio/Makefile b/sound/virtio/Makefile
index 69162a545a41..626af3cc3ed7 100644
--- a/sound/virtio/Makefile
+++ b/sound/virtio/Makefile
@@ -5,5 +5,6 @@ obj-$(CONFIG_SND_VIRTIO) += virtio_snd.o
virtio_snd-objs := \
virtio_card.o \
virtio_ctl_msg.o \
-   virtio_pcm.o
+   virtio_pcm.o \
+   virtio_pcm_msg.o

diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 39fe13b43dd1..11d025ee77c2 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -143,6 +143,12 @@ static int virtsnd_find_vqs(struct virtio_snd *snd)
callbacks[VIRTIO_SND_VQ_CONTROL] = virtsnd_ctl_notify_cb;
callbacks[VIRTIO_SND_VQ_EVENT] = virtsnd_event_notify_cb;

+   virtio_cread(vdev, struct virtio_snd_config, streams, );
+   if (n) {
+   callbacks[VIRTIO_SND_VQ_TX] = virtsnd_pcm_tx_notify_cb;
+   callbacks[VIRTIO_SND_VQ_RX] = virtsnd_pcm_rx_notify_cb;
+   }
+
rc = virtio_find_vqs(vdev, VIRTIO_SND_VQ_MAX, vqs, callbacks, names,
 NULL);
if (rc) {
@@ -177,6 +183,10 @@ static int virtsnd_find_vqs(struct virtio_snd *snd)
 * virtsnd_enable_event_vq() - Enable the event virtqueue.
 * @snd: VirtIO sound device.
 *
+ * The tx queue is enabled only if the device supports playback stream(s).
+ *
+ * The rx queue is enabled only if the device supports capture stream(s).
+ *
 * Context: Any context.
 */
static void virtsnd_enable_event_vq(struct virtio_snd *snd)
diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
index be6651a6aaf8..b11c09984882 100644
--- a/sound/virtio/virtio_card.h
+++ b/sound/virtio/virtio_card.h
@@ -89,4 +89,13 @@ virtsnd_rx_queue(struct virtio_snd *snd)
return >queues[VIRTIO_SND_VQ_RX];
}

+static inline struct virtio_snd_queue *
+virtsnd_pcm_queue(struct virtio_pcm_substream *substream)
+{
+   if (substream->direction == SNDRV_PCM_STREAM_PLAYBACK)
+   return virtsnd_tx_queue(substream->snd);
+   else
+   return virtsnd_rx_queue(substream->snd);
+}
+
#endif /* VIRTIO_SND_CARD_H */
diff --git a/sound/virtio/virtio_pcm.c b/sound/virtio/virtio_pcm.c
index 036990b7b78a..1ab50dcc88c8 100644
--- a/sound/virtio/virtio_pcm.c
+++ b/sound/virtio/virtio_pcm.c
@@ -376,6 +376,7 @@ int virtsnd_pcm_parse_cfg(struct virtio_snd *snd)

substream->snd = snd;
substream->sid = i;
+   init_waitqueue_head(>msg_empty);

rc = virtsnd_pcm_build_hw(substream, [i]);
if (rc)
@@ -530,6 +531,8 @@ void virtsnd_pcm_event(struct virtio_snd *snd, struct 
virtio_snd_event *event)
break;
}
case VIRTIO_SND_EVT_PCM_XRUN: {
+   if (atomic_read(>xfer_enabled))


Why does .xfer_enabled have to be atomic? It only takes two values - 0 and 
1, I don't see any incrementing, or test-and-set type operations or 
anything similar. Also I don't see .xfer_enabled being set to 1 anywhere 
in this patch, presumably that happens in one of later patches.



+   atomic_set(>xfer_xrun, 1);


Ditto.


break;
}
}
diff --git a/sound/virtio/virtio_pcm.h b/sound/virtio/virtio_pcm.h
index 73fb4d9dc524..d011b7e1d18d 100644
--- a/sound/virtio/virtio_pcm.h
+++ b/sound/virtio/virtio_pcm.h
@@ -24,6 +24,7 @@
#include 

struct virtio_pcm;
+struct virtio_pcm_msg;

/**
 * struct virtio_pcm_substream - VirtIO PCM substream.
@@ -34,6 +35,16 @@ struct virtio_pcm;
 * @features: Stream VirtIO feature bit map (1 << VIRTIO_SND_PCM_F_XXX).
 * @substream: Kernel ALSA substream.
 * @hw: Kernel ALSA substream hardware descriptor.
+ * @frame_bytes: Current frame size in bytes.
+ * @period_size: Current period size in frames.
+ * @buffer_size: Current buffer size in frames.
+ * @hw_ptr: Substream hardware pointer value in frames [0 ... buffer_size).
+ * @xfer_enabled: Data transfer state (0 - off, 1 - on).
+ * @xfer_xrun: 

Re: [PATCH v2 4/9] ALSA: virtio: build PCM devices and substream hardware descriptors

[Guennadi Liakhovetski]

On Sun, 24 Jan 2021, Anton Yakovlev wrote:


Like the HDA specification, the virtio sound device specification links
PCM substreams, jacks and PCM channel maps into functional groups. For
each discovered group, a PCM device is created, the number of which
coincides with the group number.

Introduce the module parameters for setting the hardware buffer
parameters:
 pcm_buffer_ms [=160]
 pcm_periods_min [=2]
 pcm_periods_max [=16]
 pcm_period_ms_min [=10]
 pcm_period_ms_max [=80]

Signed-off-by: Anton Yakovlev 
---
sound/virtio/Makefile  |   3 +-
sound/virtio/virtio_card.c |  45 
sound/virtio/virtio_card.h |   9 +
sound/virtio/virtio_pcm.c  | 536 +
sound/virtio/virtio_pcm.h  |  89 ++
5 files changed, 681 insertions(+), 1 deletion(-)
create mode 100644 sound/virtio/virtio_pcm.c
create mode 100644 sound/virtio/virtio_pcm.h

diff --git a/sound/virtio/Makefile b/sound/virtio/Makefile
index dc551e637441..69162a545a41 100644
--- a/sound/virtio/Makefile
+++ b/sound/virtio/Makefile
@@ -4,5 +4,6 @@ obj-$(CONFIG_SND_VIRTIO) += virtio_snd.o

virtio_snd-objs := \
virtio_card.o \
-   virtio_ctl_msg.o
+   virtio_ctl_msg.o \
+   virtio_pcm.o

diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 955eadc2d858..39fe13b43dd1 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -92,6 +92,17 @@ static void virtsnd_event_notify_cb(struct virtqueue *vqueue)
if (!event)
break;

+   switch (le32_to_cpu(event->hdr.code)) {
+   case VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED:
+   case VIRTIO_SND_EVT_PCM_XRUN: {


In the previous patch you had a switch-case statement complying to the 
common kernel coding style. It isn't specified in coding-style.rst, but 
these superfluous braces really don't seem to be good for anything - in 
this and multiple other switch-case statements in the series.



+   virtsnd_pcm_event(snd, event);
+   break;
+   }
+   default: {
+   break;


An empty default doesn't seem very useful either. So the above could've 
just been


+   switch (le32_to_cpu(event->hdr.code)) {
+   case VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED:
+   case VIRTIO_SND_EVT_PCM_XRUN:
+   virtsnd_pcm_event(snd, event);
+   }


+   }
+   }
+
virtsnd_event_send(queue->vqueue, event, true,
   GFP_ATOMIC);
}
@@ -274,6 +285,16 @@ static int virtsnd_build_devs(struct virtio_snd *snd)
strscpy(snd->card->longname, "VirtIO Sound Card",
sizeof(snd->card->longname));

+   rc = virtsnd_pcm_parse_cfg(snd);
+   if (rc)
+   return rc;
+
+   if (snd->nsubstreams) {
+   rc = virtsnd_pcm_build_devs(snd);
+   if (rc)
+   return rc;
+   }
+
return snd_card_register(snd->card);
}

@@ -302,6 +323,9 @@ static int virtsnd_validate(struct virtio_device *vdev)
return -EINVAL;
}

+   if (virtsnd_pcm_validate(vdev))
+   return -EINVAL;
+
return 0;
}

@@ -325,6 +349,7 @@ static int virtsnd_probe(struct virtio_device *vdev)
snd->vdev = vdev;
INIT_WORK(>reset_work, virtsnd_reset_fn);
INIT_LIST_HEAD(>ctl_msgs);
+   INIT_LIST_HEAD(>pcm_list);

vdev->priv = snd;

@@ -359,6 +384,8 @@ static int virtsnd_probe(struct virtio_device *vdev)
static void virtsnd_remove(struct virtio_device *vdev)
{
struct virtio_snd *snd = vdev->priv;
+   struct virtio_pcm *pcm;
+   struct virtio_pcm *pcm_next;

if (!snd)
return;
@@ -376,6 +403,24 @@ static void virtsnd_remove(struct virtio_device *vdev)
vdev->config->reset(vdev);
vdev->config->del_vqs(vdev);

+   list_for_each_entry_safe(pcm, pcm_next, >pcm_list, list) {
+   unsigned int i;
+
+   list_del(>list);
+
+   for (i = 0; i < ARRAY_SIZE(pcm->streams); ++i) {
+   struct virtio_pcm_stream *stream = >streams[i];
+
+   if (stream->substreams)
+   devm_kfree(>dev, stream->substreams);
+   }
+
+   devm_kfree(>dev, pcm);


Please double-check both devm_kfree() calls above. Probably they aren't 
needed in the .remove() method.



+   }
+
+   if (snd->substreams)
+   devm_kfree(>dev, snd->substreams);
+
devm_kfree(>dev, snd);

vdev->priv = NULL;
diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
index 37b734a92134..be6651a6aaf8 100644
--- a/sound/virtio/virtio_card.h

Re: [PATCH v2 3/9] ALSA: virtio: handling control messages

[Guennadi Liakhovetski]

I think the use of (devm_)kmalloc() and friends needs some refinement in 
several patches in the series.


On Sun, 24 Jan 2021, Anton Yakovlev wrote:


The control queue can be used by different parts of the driver to send
commands to the device. Control messages can be either synchronous or
asynchronous. The lifetime of a message is controlled by a reference
count.

Introduce a module parameter to set the message completion timeout:
 msg_timeout_ms [=1000]

Signed-off-by: Anton Yakovlev 
---
sound/virtio/Makefile |   3 +-
sound/virtio/virtio_card.c|  20 +++
sound/virtio/virtio_card.h|   7 +
sound/virtio/virtio_ctl_msg.c | 293 ++
sound/virtio/virtio_ctl_msg.h | 122 ++
5 files changed, 444 insertions(+), 1 deletion(-)
create mode 100644 sound/virtio/virtio_ctl_msg.c
create mode 100644 sound/virtio/virtio_ctl_msg.h


[snip]


diff --git a/sound/virtio/virtio_ctl_msg.c b/sound/virtio/virtio_ctl_msg.c
new file mode 100644
index ..c1701756bc32
--- /dev/null
+++ b/sound/virtio/virtio_ctl_msg.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Sound card driver for virtio
+ * Copyright (C) 2020  OpenSynergy GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify


Same comment about licence, and in other patches as well.


+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+#include 
+#include 
+
+#include "virtio_card.h"
+#include "virtio_ctl_msg.h"
+
+/**
+ * virtsnd_ctl_msg_alloc_ext() - Allocate and initialize a control message.
+ * @vdev: VirtIO parent device.
+ * @request_size: Size of request header (pointed to by sg_request field).
+ * @response_size: Size of response header (pointed to by sg_response field).
+ * @sgs: Additional data to attach to the message (may be NULL).
+ * @out_sgs: Number of scattergather elements to attach to the request header.
+ * @in_sgs: Number of scattergather elements to attach to the response header.
+ * @gfp: Kernel flags for memory allocation.
+ *
+ * The message will be automatically freed when the ref_count value is 0.
+ *
+ * Context: Any context. May sleep if @gfp flags permit.
+ * Return: Allocated message on success, ERR_PTR(-errno) on failure.
+ */
+struct virtio_snd_msg *virtsnd_ctl_msg_alloc_ext(struct virtio_device *vdev,
+size_t request_size,
+size_t response_size,
+struct scatterlist *sgs,
+unsigned int out_sgs,
+unsigned int in_sgs, gfp_t gfp)
+{
+   struct virtio_snd_msg *msg;
+   size_t msg_size =
+   sizeof(*msg) + (1 + out_sgs + 1 + in_sgs) * sizeof(*msg->sgs);
+   unsigned int i;
+
+   msg = devm_kzalloc(>dev, msg_size + request_size + response_size,
+  gfp);


Messages are short-lived, right? So, I think their allocation and freeing 
has to be explicit, no need for devm_.



+   if (!msg)
+   return ERR_PTR(-ENOMEM);
+
+   sg_init_one(>sg_request, (u8 *)msg + msg_size, request_size);
+   sg_init_one(>sg_response, (u8 *)msg + msg_size + request_size,
+   response_size);
+
+   INIT_LIST_HEAD(>list);
+   init_completion(>notify);
+   atomic_set(>ref_count, 1);
+
+   msg->sgs[msg->out_sgs++] = >sg_request;
+   if (sgs)
+   for (i = 0; i < out_sgs; ++i)
+   msg->sgs[msg->out_sgs++] = [i];
+
+   msg->sgs[msg->out_sgs + msg->in_sgs++] = >sg_response;
+   if (sgs)
+   for (i = out_sgs; i < out_sgs + in_sgs; ++i)
+   msg->sgs[msg->out_sgs + msg->in_sgs++] = [i];
+
+   return msg;
+}
+
+/**
+ * virtsnd_ctl_msg_send() - Send an (asynchronous) control message.
+ * @snd: VirtIO sound device.
+ * @msg: Control message.
+ *
+ * If a message is failed to be enqueued, it will be deleted. If message 
content
+ * is still needed, the caller must additionally to virtsnd_ctl_msg_ref/unref()
+ * it.
+ *
+ * Context: Any context. Takes and releases the control queue spinlock.
+ * Return: 0 on success, -errno on failure.
+ */
+int virtsnd_ctl_msg_send(struct virtio_snd *snd, struct virtio_snd_msg *msg)
+{
+   struct virtio_device *vdev = snd->vdev;
+   struct virtio_snd_queue *queue = virtsnd_control_queue(snd);

Re: [PATCH v2 2/9] ALSA: virtio: add virtio sound driver

[Guennadi Liakhovetski]

Hi Anton,

A couple of mostly cosmetic comments inline.

On Sun, 24 Jan 2021, Anton Yakovlev wrote:


Introduce skeleton of the virtio sound driver. The driver implements
the virtio sound device specification, which has become part of the
virtio standard.

Initial initialization of the device, virtqueues and creation of an
empty ALSA sound device. Also, handling DEVICE_NEEDS_RESET device
status.

Signed-off-by: Anton Yakovlev 
---
MAINTAINERS |   9 +
include/uapi/linux/virtio_snd.h | 361 +++
sound/Kconfig   |   2 +
sound/Makefile  |   3 +-
sound/virtio/Kconfig|  10 +
sound/virtio/Makefile   |   7 +
sound/virtio/virtio_card.c  | 415 
sound/virtio/virtio_card.h  |  76 ++
8 files changed, 882 insertions(+), 1 deletion(-)
create mode 100644 include/uapi/linux/virtio_snd.h
create mode 100644 sound/virtio/Kconfig
create mode 100644 sound/virtio/Makefile
create mode 100644 sound/virtio/virtio_card.c
create mode 100644 sound/virtio/virtio_card.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 00836f6452f0..3f509d54a457 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18936,6 +18936,15 @@ W: https://virtio-mem.gitlab.io/
F:  drivers/virtio/virtio_mem.c
F:  include/uapi/linux/virtio_mem.h

+VIRTIO SOUND DRIVER
+M: Anton Yakovlev 
+M: "Michael S. Tsirkin" 
+L: virtualization@lists.linux-foundation.org
+L: alsa-de...@alsa-project.org (moderated for non-subscribers)
+S: Maintained
+F: include/uapi/linux/virtio_snd.h
+F: sound/virtio/*
+
VIRTUAL BOX GUEST DEVICE DRIVER
M:  Hans de Goede 
M:  Arnd Bergmann 
diff --git a/include/uapi/linux/virtio_snd.h b/include/uapi/linux/virtio_snd.h
new file mode 100644
index ..1ff6310e54d6
--- /dev/null
+++ b/include/uapi/linux/virtio_snd.h
@@ -0,0 +1,361 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Copyright (C) 2020  OpenSynergy GmbH
+ *
+ * This header is BSD licensed so anyone can use the definitions to
+ * implement compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:


Can a BSD licence actually be further restricted?


+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of OpenSynergy GmbH nor the names of its contributors
+ *may be used to endorse or promote products derived from this software
+ *without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR


IBM? Also no idea whether this warranty disclaimer is appropriate here. I 
thought we were transitioning to those SPDX identifiers to eliminate all 
these headers.



+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */


[snip]


diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
new file mode 100644
index ..532d823fdf6f
--- /dev/null
+++ b/sound/virtio/virtio_card.c
@@ -0,0 +1,415 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Sound card driver for virtio
+ * Copyright (C) 2020  OpenSynergy GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.


Same here, I think SPDX means you don't need all this here any more.


+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "virtio_card.h"
+

Re: [PATCH v3 2/4] drm/qxl: unpin release objects

[Gerd Hoffmann]

> > Just calling ttm_bo_unpin() here makes lockdep unhappy.
> 
> How does that one splat? But yeah if that's a problem should be
> explained in the comment. I'd then also only do a pin_count--; to make
> sure you can still catch other pin leaks if you have them. Setting it
> to 0 kinda defeats the warning.

Figured the unpin is at the completely wrong place while trying to
reproduce the lockdep splat ...

take care,
  Gerd

>From 43befab4a935114e8620af62781666fa81288255 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann 
Date: Mon, 25 Jan 2021 13:10:50 +0100
Subject: [PATCH] drm/qxl: unpin release objects

Balances the qxl_create_bo(..., pinned=true, ...);
call in qxl_release_bo_alloc().

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_release.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/qxl/qxl_release.c 
b/drivers/gpu/drm/qxl/qxl_release.c
index c52412724c26..28013fd1f8ea 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -347,6 +347,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev, 
unsigned long size,
 
mutex_lock(>release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >= 
releases_per_bo[cur_idx]) {
+   qxl_bo_unpin(qdev->current_release_bo[cur_idx]);
qxl_bo_unref(>current_release_bo[cur_idx]);
qdev->current_release_bo_offset[cur_idx] = 0;
qdev->current_release_bo[cur_idx] = NULL;
-- 
2.29.2

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH][next] vpda: Fix memory leaks of msg on error return paths

[Stefano Garzarella]

On Fri, Jan 22, 2021 at 02:52:35PM +, Colin King wrote:

From: Colin Ian King 

There are two error return paths that neglect to free the allocated
object msg that lead to memory leaks. Fix this by adding an error
exit path that frees msg.

Addresses-Coverity: ("Resource leak")
Fixes: 39502d042a70 ("vdpa: Enable user to query vdpa device info")
Signed-off-by: Colin Ian King 
---
drivers/vdpa/vdpa.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 9700a0adcca0..eb1f5a514103 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -540,13 +540,15 @@ static int vdpa_nl_cmd_dev_get_doit(struct sk_buff *skb, 
struct genl_info *info)
if (!dev) {
mutex_unlock(_dev_mutex);
NL_SET_ERR_MSG_MOD(info->extack, "device not found");
-   return -ENODEV;
+   err = -ENODEV;
+   goto err;
}
vdev = container_of(dev, struct vdpa_device, dev);
if (!vdev->mdev) {
mutex_unlock(_dev_mutex);
put_device(dev);
-   return -EINVAL;
+   err = -EINVAL;
+   goto err;
}
err = vdpa_dev_fill(vdev, msg, info->snd_portid, info->snd_seq, 0, 
info->extack);
if (!err)
@@ -554,6 +556,7 @@ static int vdpa_nl_cmd_dev_get_doit(struct sk_buff *skb, 
struct genl_info *info)
put_device(dev);
mutex_unlock(_dev_mutex);

+err:
if (err)
nlmsg_free(msg);
return err;


The patch looks okay, but reviewing it I figure out that if 
genlmsg_reply() returns an error, it also frees the sk_buff passed, so 
IIUC calling nlmsg_free() when genlmsg_reply() fails should cause a 
double free.


Maybe we should do something like this (not tested):

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 9700a0adcca0..920afcb4aa75 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -538,24 +538,29 @@ static int vdpa_nl_cmd_dev_get_doit(struct sk_buff *skb, 
struct genl_info *info)
mutex_lock(_dev_mutex);
dev = bus_find_device(_bus, NULL, devname, vdpa_name_match);
if (!dev) {
-   mutex_unlock(_dev_mutex);
NL_SET_ERR_MSG_MOD(info->extack, "device not found");
-   return -ENODEV;
+   err= -ENODEV;
+   goto err_msg;
}
vdev = container_of(dev, struct vdpa_device, dev);
if (!vdev->mdev) {
-   mutex_unlock(_dev_mutex);
-   put_device(dev);
-   return -EINVAL;
+   err = -EINVAL;
+   goto err_dev;
}
err = vdpa_dev_fill(vdev, msg, info->snd_portid, info->snd_seq, 0, 
info->extack);
-   if (!err)
-   err = genlmsg_reply(msg, info);
+   if (err)
+   goto err_dev;
+
put_device(dev);
mutex_unlock(_dev_mutex);
 
-   if (err)

-   nlmsg_free(msg);
+   return genlmsg_reply(msg, info);
+
+err_dev:
+   put_device(dev);
+err_msg:
+   mutex_unlock(_dev_mutex);
+   nlmsg_free(msg);
return err;
 }
 


Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v1 00/15] iommu/virtio: Nested stage support with Arm

[Auger Eric]

Hi Vivek,

On 1/21/21 6:34 PM, Vivek Kumar Gautam wrote:
> Hi Eric,
> 
> 
> On 1/19/21 2:33 PM, Auger Eric wrote:
>> Hi Vivek,
>>
>> On 1/15/21 1:13 PM, Vivek Gautam wrote:
>>> This patch-series aims at enabling Nested stage translation in guests
>>> using virtio-iommu as the paravirtualized iommu. The backend is
>>> supported
>>> with Arm SMMU-v3 that provides nested stage-1 and stage-2 translation.
>>>
>>> This series derives its purpose from various efforts happening to add
>>> support for Shared Virtual Addressing (SVA) in host and guest. On Arm,
>>> most of the support for SVA has already landed. The support for nested
>>> stage translation and fault reporting to guest has been proposed [1].
>>> The related changes required in VFIO [2] framework have also been put
>>> forward.
>>>
>>> This series proposes changes in virtio-iommu to program PASID tables
>>> and related stage-1 page tables. A simple iommu-pasid-table library
>>> is added for this purpose that interacts with vendor drivers to
>>> allocate and populate PASID tables.
>>> In Arm SMMUv3 we propose to pull the Context Descriptor (CD) management
>>> code out of the arm-smmu-v3 driver and add that as a glue vendor layer
>>> to support allocating CD tables, and populating them with right values.
>>> These CD tables are essentially the PASID tables and contain stage-1
>>> page table configurations too.
>>> A request to setup these CD tables come from virtio-iommu driver using
>>> the iommu-pasid-table library when running on Arm. The virtio-iommu
>>> then pass these PASID tables to the host using the right virtio backend
>>> and support in VMM.
>>>
>>> For testing we have added necessary support in kvmtool. The changes in
>>> kvmtool are based on virtio-iommu development branch by Jean-Philippe
>>> Brucker [3].
>>>
>>> The tested kernel branch contains following in the order bottom to top
>>> on the git hash -
>>> a) v5.11-rc3
>>> b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
>>>     table support for Arm.
>>> c) Smmu test engine patches from Jean-Philippe's branch [4]
>>> d) This series
>>> e) Domain nesting info patches [5][6][7].
>>> f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
>>>     the list).
>>>
>>> This kernel is tested on Neoverse reference software stack with
>>> Fixed virtual platform. Public version of the software stack and
>>> FVP is available here[8][9].
>>>
>>> A big thanks to Jean-Philippe for his contributions towards this work
>>> and for his valuable guidance.
>>>
>>> [1]
>>> https://lore.kernel.org/linux-iommu/20201118112151.25412-1-eric.au...@redhat.com/T/
>>>
>>> [2]
>>> https://lore.kernel.org/kvmarm/20201116110030.32335-12-eric.au...@redhat.com/T/
>>>
>>> [3] https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
>>> [4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
>>> [5]
>>> https://lore.kernel.org/kvm/1599734733-6431-2-git-send-email-yi.l@intel.com/
>>>
>>> [6]
>>> https://lore.kernel.org/kvm/1599734733-6431-3-git-send-email-yi.l@intel.com/
>>>
>>> [7]
>>> https://lore.kernel.org/kvm/1599734733-6431-4-git-send-email-yi.l@intel.com/
>>>
>>> [8]
>>> https://developer.arm.com/tools-and-software/open-source-software/arm-platforms-software/arm-ecosystem-fvps
>>>
>>> [9]
>>> https://git.linaro.org/landing-teams/working/arm/arm-reference-platforms.git/about/docs/rdn1edge/user-guide.rst
>>>
>>
>> Could you share a public branch where we could find all the kernel
>> pieces.
>>
>> Thank you in advance
> 
> Apologies for the delay. It took a bit of time to sort things out for a
> public branch.
> The branch is available in my github now. Please have a look.
> 
> https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-virtio-iommu

no problem. Thank you for the link.

Best Regards

Eric
> 
> 
> 
> Thanks and regards
> Vivek
> 
>>
>> Best Regards
>>
>> Eric
>>>
>>> Jean-Philippe Brucker (6):
>>>    iommu/virtio: Add headers for table format probing
>>>    iommu/virtio: Add table format probing
>>>    iommu/virtio: Add headers for binding pasid table in iommu
>>>    iommu/virtio: Add support for INVALIDATE request
>>>    iommu/virtio: Attach Arm PASID tables when available
>>>    iommu/virtio: Add support for Arm LPAE page table format
>>>
>>> Vivek Gautam (9):
>>>    iommu/arm-smmu-v3: Create a Context Descriptor library
>>>    iommu: Add a simple PASID table library
>>>    iommu/arm-smmu-v3: Update drivers to work with iommu-pasid-table
>>>    iommu/arm-smmu-v3: Update CD base address info for user-space
>>>    iommu/arm-smmu-v3: Set sync op from consumer driver of cd-lib
>>>    iommu: Add asid_bits to arm smmu-v3 stage1 table info
>>>    iommu/virtio: Update table format probing header
>>>    iommu/virtio: Prepare to add attach pasid table infrastructure
>>>    iommu/virtio: Update fault type and reason info for viommu fault
>>>
>>>   drivers/iommu/arm/arm-smmu-v3/Makefile    |   2 +-
>>>   .../arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c  | 283