RE: [PATCH V3 3/5] hyperv/IOMMU: Enable swiotlb bounce buffer for Isolation VM

["Michael Kelley (LINUX)" <mikelley@xxxxxxxxxxxxx>]

From: Tianyu Lan <ltykernel@xxxxxxxxx> Sent: Wednesday, December 1, 2021 8:03 AM
> 
> hyperv Isolation VM requires bounce buffer support to copy
> data from/to encrypted memory and so enable swiotlb force
> mode to use swiotlb bounce buffer for DMA transaction.
> 
> In Isolation VM with AMD SEV, the bounce buffer needs to be
> accessed via extra address space which is above shared_gpa_boundary
> (E.G 39 bit address line) reported by Hyper-V CPUID ISOLATION_CONFIG.
> The access physical address will be original physical address +
> shared_gpa_boundary. The shared_gpa_boundary in the AMD SEV SNP
> spec is called virtual top of memory(vTOM). Memory addresses below
> vTOM are automatically treated as private while memory above
> vTOM is treated as shared.
> 
> Hyper-V initalizes swiotlb bounce buffer and default swiotlb
> needs to be disabled. pci_swiotlb_detect_override() and
> pci_swiotlb_detect_4gb() enable the default one. To override
> the setting, hyperv_swiotlb_detect() needs to run before
> these detect functions which depends on the pci_xen_swiotlb_
> init(). Make pci_xen_swiotlb_init() depends on the hyperv_swiotlb
> _detect() to keep the order.
> 
> Swiotlb bounce buffer code calls set_memory_decrypted()
> to mark bounce buffer visible to host and map it in extra
> address space via memremap. Populate the shared_gpa_boundary
> (vTOM) via swiotlb_unencrypted_base variable.
> 
> The map function memremap() can't work in the early place
> hyperv_iommu_swiotlb_init() and so call swiotlb_update_mem_attributes()
> in the hyperv_iommu_swiotlb_later_init().
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>
> ---
>  arch/x86/xen/pci-swiotlb-xen.c |  3 +-
>  drivers/hv/vmbus_drv.c         |  3 ++
>  drivers/iommu/hyperv-iommu.c   | 56 ++++++++++++++++++++++++++++++++++
>  include/linux/hyperv.h         |  8 +++++
>  4 files changed, 69 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
> index 46df59aeaa06..30fd0600b008 100644
> --- a/arch/x86/xen/pci-swiotlb-xen.c
> +++ b/arch/x86/xen/pci-swiotlb-xen.c
> @@ -4,6 +4,7 @@
> 
>  #include <linux/dma-map-ops.h>
>  #include <linux/pci.h>
> +#include <linux/hyperv.h>
>  #include <xen/swiotlb-xen.h>
> 
>  #include <asm/xen/hypervisor.h>
> @@ -91,6 +92,6 @@ int pci_xen_swiotlb_init_late(void)
>  EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
> 
>  IOMMU_INIT_FINISH(pci_xen_swiotlb_detect,
> -               NULL,
> +               hyperv_swiotlb_detect,
>                 pci_xen_swiotlb_init,
>                 NULL);
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 392c1ac4f819..0a64ccfafb8b 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -33,6 +33,7 @@
>  #include <linux/random.h>
>  #include <linux/kernel.h>
>  #include <linux/syscore_ops.h>
> +#include <linux/dma-map-ops.h>
>  #include <clocksource/hyperv_timer.h>
>  #include "hyperv_vmbus.h"
> 
> @@ -2078,6 +2079,7 @@ struct hv_device *vmbus_device_create(const guid_t 
> *type,
>       return child_device_obj;
>  }
> 
> +static u64 vmbus_dma_mask = DMA_BIT_MASK(64);
>  /*
>   * vmbus_device_register - Register the child device
>   */
> @@ -2118,6 +2120,7 @@ int vmbus_device_register(struct hv_device 
> *child_device_obj)
>       }
>       hv_debug_add_dev_dir(child_device_obj);
> 
> +     child_device_obj->device.dma_mask = &vmbus_dma_mask;
>       return 0;
> 
>  err_kset_unregister:
> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> index e285a220c913..dd729d49a1eb 100644
> --- a/drivers/iommu/hyperv-iommu.c
> +++ b/drivers/iommu/hyperv-iommu.c
> @@ -13,14 +13,20 @@
>  #include <linux/irq.h>
>  #include <linux/iommu.h>
>  #include <linux/module.h>
> +#include <linux/hyperv.h>
> +#include <linux/io.h>
> 
>  #include <asm/apic.h>
>  #include <asm/cpu.h>
>  #include <asm/hw_irq.h>
>  #include <asm/io_apic.h>
> +#include <asm/iommu.h>
> +#include <asm/iommu_table.h>
>  #include <asm/irq_remapping.h>
>  #include <asm/hypervisor.h>
>  #include <asm/mshyperv.h>
> +#include <asm/swiotlb.h>
> +#include <linux/dma-direct.h>
> 
>  #include "irq_remapping.h"
> 
> @@ -337,4 +343,54 @@ static const struct irq_domain_ops 
> hyperv_root_ir_domain_ops = {
>       .free = hyperv_root_irq_remapping_free,
>  };
> 
> +static void __init hyperv_iommu_swiotlb_init(void)
> +{
> +     unsigned long hyperv_io_tlb_size;
> +     void *hyperv_io_tlb_start;
> +
> +     /*
> +      * Allocate Hyper-V swiotlb bounce buffer at early place
> +      * to reserve large contiguous memory.
> +      */
> +     hyperv_io_tlb_size = swiotlb_size_or_default();
> +     hyperv_io_tlb_start = memblock_alloc(hyperv_io_tlb_size, PAGE_SIZE);
> +
> +     if (!hyperv_io_tlb_start)
> +             pr_warn("Fail to allocate Hyper-V swiotlb buffer.\n");

In the error case, won't swiotlb_init_with_tlb() end up panic'ing when
it tries to zero out the memory?   The only real choice here is to
return immediately after printing the message, and not call
swiotlb_init_with_tlb().

> +
> +     swiotlb_init_with_tbl(hyperv_io_tlb_start,
> +                           hyperv_io_tlb_size >> IO_TLB_SHIFT, true);
> +}
> +
> +int __init hyperv_swiotlb_detect(void)
> +{
> +     if (!hypervisor_is_type(X86_HYPER_MS_HYPERV))
> +             return 0;
> +
> +     if (!hv_is_isolation_supported())
> +             return 0;
> +
> +     /*
> +      * Enable swiotlb force mode in Isolation VM to
> +      * use swiotlb bounce buffer for dma transaction.
> +      */
> +     if (hv_isolation_type_snp())
> +             swiotlb_unencrypted_base = ms_hyperv.shared_gpa_boundary;
> +     swiotlb_force = SWIOTLB_FORCE;
> +     return 1;
> +}
> +
> +static void __init hyperv_iommu_swiotlb_later_init(void)
> +{
> +     /*
> +      * Swiotlb bounce buffer needs to be mapped in extra address
> +      * space. Map function doesn't work in the early place and so
> +      * call swiotlb_update_mem_attributes() here.
> +      */
> +     swiotlb_update_mem_attributes();
> +}
> +
> +IOMMU_INIT_FINISH(hyperv_swiotlb_detect,
> +               NULL, hyperv_iommu_swiotlb_init,
> +               hyperv_iommu_swiotlb_later_init);
>  #endif
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index b823311eac79..1f037e114dc8 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1726,6 +1726,14 @@ int hyperv_write_cfg_blk(struct pci_dev *dev, void 
> *buf, unsigned int len,
>  int hyperv_reg_block_invalidate(struct pci_dev *dev, void *context,
>                               void (*block_invalidate)(void *context,
>                                                        u64 block_mask));
> +#if IS_ENABLED(CONFIG_HYPERV)
> +int __init hyperv_swiotlb_detect(void);
> +#else
> +static inline int __init hyperv_swiotlb_detect(void)
> +{
> +     return 0;
> +}
> +#endif
> 
>  struct hyperv_pci_block_ops {
>       int (*read_block)(struct pci_dev *dev, void *buf, unsigned int buf_len,
> --
> 2.25.1