diff mbox series

[V6,3/5] hyper-v: Enable swiotlb bounce buffer for Isolation VM

Message ID 20211207075602.2452-4-ltykernel@gmail.com (mailing list archive)
State Superseded
Headers show
Series x86/Hyper-V: Add Hyper-V Isolation VM support(Second part) | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Tianyu Lan Dec. 7, 2021, 7:55 a.m. UTC 
From: Tianyu Lan <Tianyu.Lan@microsoft.com>

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.

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
(e.g ms_hyperv_init_platform()) and so call swiotlb_update_mem_
attributes() in the hyperv_init().

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v4:
	* Remove Hyper-V IOMMU IOMMU_INIT_FINISH related functions
	  and set SWIOTLB_FORCE and swiotlb_unencrypted_base in the
	  ms_hyperv_init_platform(). Call swiotlb_update_mem_attributes()
	  in the hyperv_init().

Change since v3:
	* Add comment in pci-swiotlb-xen.c to explain why add
	  dependency between hyperv_swiotlb_detect() and pci_
	  xen_swiotlb_detect().
	* Return directly when fails to allocate Hyper-V swiotlb
	  buffer in the hyperv_iommu_swiotlb_init().
---
 arch/x86/hyperv/hv_init.c      | 10 ++++++++++
 arch/x86/kernel/cpu/mshyperv.c | 11 ++++++++++-
 include/linux/hyperv.h         |  8 ++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

Comments

Michael Kelley (LINUX) Dec. 9, 2021, 8:09 p.m. UTC | #1 
From: Tianyu Lan <ltykernel@gmail.com> Sent: Monday, December 6, 2021 11:56 PM
> 
> 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.
> 
> 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
> (e.g ms_hyperv_init_platform()) and so call swiotlb_update_mem_
> attributes() in the hyperv_init().
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v4:
> 	* Remove Hyper-V IOMMU IOMMU_INIT_FINISH related functions
> 	  and set SWIOTLB_FORCE and swiotlb_unencrypted_base in the
> 	  ms_hyperv_init_platform(). Call swiotlb_update_mem_attributes()
> 	  in the hyperv_init().
> 
> Change since v3:
> 	* Add comment in pci-swiotlb-xen.c to explain why add
> 	  dependency between hyperv_swiotlb_detect() and pci_
> 	  xen_swiotlb_detect().
> 	* Return directly when fails to allocate Hyper-V swiotlb
> 	  buffer in the hyperv_iommu_swiotlb_init().
> ---
>  arch/x86/hyperv/hv_init.c      | 10 ++++++++++
>  arch/x86/kernel/cpu/mshyperv.c | 11 ++++++++++-
>  include/linux/hyperv.h         |  8 ++++++++
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 24f4a06ac46a..9e18a280f89d 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -28,6 +28,7 @@
>  #include <linux/syscore_ops.h>
>  #include <clocksource/hyperv_timer.h>
>  #include <linux/highmem.h>
> +#include <linux/swiotlb.h>
> 
>  int hyperv_init_cpuhp;
>  u64 hv_current_partition_id = ~0ull;
> @@ -502,6 +503,15 @@ void __init hyperv_init(void)
> 
>  	/* Query the VMs extended capability once, so that it can be cached. */
>  	hv_query_ext_cap(0);
> +
> +	/*
> +	 * 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.
> +	 */
> +	if (hv_is_isolation_supported())
> +		swiotlb_update_mem_attributes();
> +
>  	return;
> 
>  clean_guest_os_id:
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 4794b716ec79..baf3a0873552 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -18,6 +18,7 @@
>  #include <linux/kexec.h>
>  #include <linux/i8253.h>
>  #include <linux/random.h>
> +#include <linux/swiotlb.h>
>  #include <asm/processor.h>
>  #include <asm/hypervisor.h>
>  #include <asm/hyperv-tlfs.h>
> @@ -319,8 +320,16 @@ static void __init ms_hyperv_init_platform(void)
>  		pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n",
>  			ms_hyperv.isolation_config_a, ms_hyperv.isolation_config_b);
> 
> -		if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP)
> +		if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP) {
>  			static_branch_enable(&isolation_type_snp);
> +			swiotlb_unencrypted_base = ms_hyperv.shared_gpa_boundary;
> +		}
> +
> +		/*
> +		 * Enable swiotlb force mode in Isolation VM to
> +		 * use swiotlb bounce buffer for dma transaction.
> +		 */
> +		swiotlb_force = SWIOTLB_FORCE;

I'm good with this approach that directly updates the swiotlb settings here
rather than in IOMMU initialization code.  It's a lot more straightforward.

However, there's an issue if building for X86_32 without PAE, in that the 
swiotlb module may not be built, resulting in compile and link errors.  The
swiotlb.h file needs to be updated to provide a stub function for
swiotlb_update_mem_attributes().   swiotlb_unencrypted_base probably
needs wrapper functions to get/set it, which can be stubs when 
CONFIG_SWIOTLB is not set.  swiotlb_force is a bit of a mess in that it already
has a stub definition that assumes it will only be read, and not set.  A bit of
thinking will be needed to sort that out.

>  	}
> 
>  	if (hv_max_functions_eax >= HYPERV_CPUID_NESTED_FEATURES) {
> 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

I don't think hyperv_swiotlb_detect() is used any longer, so this change
should be dropped.

> 
>  struct hyperv_pci_block_ops {
>  	int (*read_block)(struct pci_dev *dev, void *buf, unsigned int buf_len,
> --
> 2.25.1
Tianyu Lan Dec. 10, 2021, 1:25 p.m. UTC | #2 
On 12/10/2021 4:09 AM, Michael Kelley (LINUX) wrote:
>> @@ -319,8 +320,16 @@ static void __init ms_hyperv_init_platform(void)
>>   		pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n",
>>   			ms_hyperv.isolation_config_a, ms_hyperv.isolation_config_b);
>>
>> -		if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP)
>> +		if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP) {
>>   			static_branch_enable(&isolation_type_snp);
>> +			swiotlb_unencrypted_base = ms_hyperv.shared_gpa_boundary;
>> +		}
>> +
>> +		/*
>> +		 * Enable swiotlb force mode in Isolation VM to
>> +		 * use swiotlb bounce buffer for dma transaction.
>> +		 */
>> +		swiotlb_force = SWIOTLB_FORCE;
> I'm good with this approach that directly updates the swiotlb settings here
> 
> rather than in IOMMU initialization code.  It's a lot more straightforward.
> 
> However, there's an issue if building for X86_32 without PAE, in that the
> swiotlb module may not be built, resulting in compile and link errors.  The
> swiotlb.h file needs to be updated to provide a stub function for
> swiotlb_update_mem_attributes().   swiotlb_unencrypted_base probably
> needs wrapper functions to get/set it, which can be stubs when
> CONFIG_SWIOTLB is not set.  swiotlb_force is a bit of a mess in that it already
> has a stub definition that assumes it will only be read, and not set.  A bit of
> thinking will be needed to sort that out.

It's ok to fix the issue via selecting swiotlb when CONFIG_HYPERV is
set?

> 
>>   	}
>>
>>   	if (hv_max_functions_eax >= HYPERV_CPUID_NESTED_FEATURES) {
>> 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
> I don't think hyperv_swiotlb_detect() is used any longer, so this change
> should be dropped.
Yes, will update.
Tianyu Lan Dec. 10, 2021, 2:01 p.m. UTC | #3 
On 12/10/2021 9:25 PM, Tianyu Lan wrote:
>>> @@ -319,8 +320,16 @@ static void __init ms_hyperv_init_platform(void)
>>>           pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 
>>> 0x%x\n",
>>>               ms_hyperv.isolation_config_a, 
>>> ms_hyperv.isolation_config_b);
>>>
>>> -        if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP)
>>> +        if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP) {
>>>               static_branch_enable(&isolation_type_snp);
>>> +            swiotlb_unencrypted_base = ms_hyperv.shared_gpa_boundary;
>>> +        }
>>> +
>>> +        /*
>>> +         * Enable swiotlb force mode in Isolation VM to
>>> +         * use swiotlb bounce buffer for dma transaction.
>>> +         */
>>> +        swiotlb_force = SWIOTLB_FORCE;
>> I'm good with this approach that directly updates the swiotlb settings 
>> here
>>
>> rather than in IOMMU initialization code.  It's a lot more 
>> straightforward.
>>
>> However, there's an issue if building for X86_32 without PAE, in that the
>> swiotlb module may not be built, resulting in compile and link 
>> errors.  The
>> swiotlb.h file needs to be updated to provide a stub function for
>> swiotlb_update_mem_attributes().   swiotlb_unencrypted_base probably
>> needs wrapper functions to get/set it, which can be stubs when
>> CONFIG_SWIOTLB is not set.  swiotlb_force is a bit of a mess in that 
>> it already
>> has a stub definition that assumes it will only be read, and not set.  
>> A bit of
>> thinking will be needed to sort that out.
> 
> It's ok to fix the issue via selecting swiotlb when CONFIG_HYPERV is
> set?
> 
Sorry. ignore the previous statement. These codes doesn't depend on 
CONFIG_HYPERV.

How about making these code under #ifdef CONFIG_X86_64 or CONFIG_SWIOTLB?
diff mbox series

Patch

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 24f4a06ac46a..9e18a280f89d 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -28,6 +28,7 @@ 
 #include <linux/syscore_ops.h>
 #include <clocksource/hyperv_timer.h>
 #include <linux/highmem.h>
+#include <linux/swiotlb.h>
 
 int hyperv_init_cpuhp;
 u64 hv_current_partition_id = ~0ull;
@@ -502,6 +503,15 @@  void __init hyperv_init(void)
 
 	/* Query the VMs extended capability once, so that it can be cached. */
 	hv_query_ext_cap(0);
+
+	/*
+	 * 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.
+	 */
+	if (hv_is_isolation_supported())
+		swiotlb_update_mem_attributes();
+
 	return;
 
 clean_guest_os_id:
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 4794b716ec79..baf3a0873552 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -18,6 +18,7 @@ 
 #include <linux/kexec.h>
 #include <linux/i8253.h>
 #include <linux/random.h>
+#include <linux/swiotlb.h>
 #include <asm/processor.h>
 #include <asm/hypervisor.h>
 #include <asm/hyperv-tlfs.h>
@@ -319,8 +320,16 @@  static void __init ms_hyperv_init_platform(void)
 		pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n",
 			ms_hyperv.isolation_config_a, ms_hyperv.isolation_config_b);
 
-		if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP)
+		if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP) {
 			static_branch_enable(&isolation_type_snp);
+			swiotlb_unencrypted_base = ms_hyperv.shared_gpa_boundary;
+		}
+
+		/*
+		 * Enable swiotlb force mode in Isolation VM to
+		 * use swiotlb bounce buffer for dma transaction.
+		 */
+		swiotlb_force = SWIOTLB_FORCE;
 	}
 
 	if (hv_max_functions_eax >= HYPERV_CPUID_NESTED_FEATURES) {
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,