Re: [PATCH hyperv-next v4 12/16] Drivers: hv: Allocate encrypted buffers when requested

[Tianyu Lan <ltykernel@gmail.com>]

On Tue, Jul 15, 2025 at 6:28 AM Roman Kisel <romank@linux.microsoft.com> wrote:
>
> Confidential VMBus is built around using buffers not shared with
> the host.
>
> Support allocating encrypted buffers when requested.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---

Reviewed-by: Tianyu Lan <tiala@microsoft.com>

>  drivers/hv/channel.c      | 49 +++++++++++++++++++++++----------------
>  drivers/hv/hyperv_vmbus.h |  3 ++-
>  drivers/hv/ring_buffer.c  |  5 ++--
>  3 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 35f26fa1ffe7..051eeba800f2 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -443,20 +443,23 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>                 return ret;
>         }
>
> -       /*
> -        * Set the "decrypted" flag to true for the set_memory_decrypted()
> -        * success case. In the failure case, the encryption state of the
> -        * memory is unknown. Leave "decrypted" as true to ensure the
> -        * memory will be leaked instead of going back on the free list.
> -        */
> -       gpadl->decrypted = true;
> -       ret = set_memory_decrypted((unsigned long)kbuffer,
> -                                  PFN_UP(size));
> -       if (ret) {
> -               dev_warn(&channel->device_obj->device,
> -                        "Failed to set host visibility for new GPADL %d.\n",
> -                        ret);
> -               return ret;
> +       gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
> +               (channel->co_ring_buffer && type == HV_GPADL_RING));
> +       if (gpadl->decrypted) {
> +               /*
> +                * The "decrypted" flag being true assumes that set_memory_decrypted() succeeds.
> +                * But if it fails, the encryption state of the memory is unknown. In that case,
> +                * leave "decrypted" as true to ensure the memory is leaked instead of going back
> +                * on the free list.
> +                */
> +               ret = set_memory_decrypted((unsigned long)kbuffer,
> +                                       PFN_UP(size));
> +               if (ret) {
> +                       dev_warn(&channel->device_obj->device,
> +                               "Failed to set host visibility for new GPADL %d.\n",
> +                               ret);
> +                       return ret;
> +               }
>         }
>
>         init_completion(&msginfo->waitevent);
> @@ -544,8 +547,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>                  * left as true so the memory is leaked instead of being
>                  * put back on the free list.
>                  */
> -               if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
> -                       gpadl->decrypted = false;
> +               if (gpadl->decrypted) {
> +                       if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
> +                               gpadl->decrypted = false;
> +               }
>         }
>
>         return ret;
> @@ -676,12 +681,13 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
>                 goto error_clean_ring;
>
>         err = hv_ringbuffer_init(&newchannel->outbound,
> -                                page, send_pages, 0);
> +                                page, send_pages, 0, newchannel->co_ring_buffer);
>         if (err)
>                 goto error_free_gpadl;
>
>         err = hv_ringbuffer_init(&newchannel->inbound, &page[send_pages],
> -                                recv_pages, newchannel->max_pkt_size);
> +                                recv_pages, newchannel->max_pkt_size,
> +                                newchannel->co_ring_buffer);
>         if (err)
>                 goto error_free_gpadl;
>
> @@ -862,8 +868,11 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, struct vmbus_gpadl *gpad
>
>         kfree(info);
>
> -       ret = set_memory_encrypted((unsigned long)gpadl->buffer,
> -                                  PFN_UP(gpadl->size));
> +       if (gpadl->decrypted)
> +               ret = set_memory_encrypted((unsigned long)gpadl->buffer,
> +                                       PFN_UP(gpadl->size));
> +       else
> +               ret = 0;
>         if (ret)
>                 pr_warn("Fail to set mem host visibility in GPADL teardown %d.\n", ret);
>
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 2873703d08a9..beae68a70939 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -200,7 +200,8 @@ extern int hv_synic_cleanup(unsigned int cpu);
>  void hv_ringbuffer_pre_init(struct vmbus_channel *channel);
>
>  int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
> -                      struct page *pages, u32 pagecnt, u32 max_pkt_size);
> +                      struct page *pages, u32 pagecnt, u32 max_pkt_size,
> +                          bool confidential);
>
>  void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info);
>
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 3c9b02471760..05c2cd42fc75 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -183,7 +183,8 @@ void hv_ringbuffer_pre_init(struct vmbus_channel *channel)
>
>  /* Initialize the ring buffer. */
>  int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
> -                      struct page *pages, u32 page_cnt, u32 max_pkt_size)
> +                      struct page *pages, u32 page_cnt, u32 max_pkt_size,
> +                          bool confidential)
>  {
>         struct page **pages_wraparound;
>         int i;
> @@ -207,7 +208,7 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
>
>         ring_info->ring_buffer = (struct hv_ring_buffer *)
>                 vmap(pages_wraparound, page_cnt * 2 - 1, VM_MAP,
> -                       pgprot_decrypted(PAGE_KERNEL));
> +                       confidential ? PAGE_KERNEL : pgprot_decrypted(PAGE_KERNEL));
>
>         kfree(pages_wraparound);
>         if (!ring_info->ring_buffer)
> --
> 2.43.0
>
>


-- 
Thanks
Tianyu Lan