ptp: kvm: Use decrypted memory in confidential guest on x86

Message ID 20230220130235.2603366-1-jpiotrowski@linux.microsoft.com
State New
Headers
Series ptp: kvm: Use decrypted memory in confidential guest on x86 |

Commit Message

Jeremi Piotrowski Feb. 20, 2023, 1:02 p.m. UTC
  KVM_HC_CLOCK_PAIRING currently fails inside SEV-SNP guests because the guest
passes an address to static data to the host. In confidential computing the
host can't access arbitrary guest memory so handling the hypercall runs into an
"rmpfault". To make the hypercall work, the guest needs to explicitly mark the
memory as decrypted. Do that in kvm_arch_ptp_init(), but retain the previous
behavior for non-confidential guests to save us from having to allocate memory
when not needed.

Add a new arch-specific function (kvm_arch_ptp_exit()) to free the
allocation and mark the memory as encrypted again.

Signed-off-by: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
---
Hi,

I would love to not allocate a whole page just for this driver, swiotlb is
decrypted but I don't have access to a 'struct device' here. Does anyone have
any suggestion?

Jeremi

 drivers/ptp/ptp_kvm_arm.c    |  4 +++
 drivers/ptp/ptp_kvm_common.c |  1 +
 drivers/ptp/ptp_kvm_x86.c    | 59 +++++++++++++++++++++++++++++-------
 3 files changed, 53 insertions(+), 11 deletions(-)
  

Comments

kernel test robot Feb. 21, 2023, 1:08 a.m. UTC | #1
Hi Jeremi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on horms-ipvs/master]
[also build test WARNING on mst-vhost/linux-next net/master net-next/master linus/master v6.2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeremi-Piotrowski/ptp-kvm-Use-decrypted-memory-in-confidential-guest-on-x86/20230220-210441
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
patch link:    https://lore.kernel.org/r/20230220130235.2603366-1-jpiotrowski%40linux.microsoft.com
patch subject: [PATCH] ptp: kvm: Use decrypted memory in confidential guest on x86
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230221/202302210943.Xq84rrhU-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/0dd1701fd254692af3d0ca051e092e8dcef190c4
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jeremi-Piotrowski/ptp-kvm-Use-decrypted-memory-in-confidential-guest-on-x86/20230220-210441
        git checkout 0dd1701fd254692af3d0ca051e092e8dcef190c4
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302210943.Xq84rrhU-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/ptp/ptp_kvm_x86.c: In function 'kvm_arch_ptp_init':
   drivers/ptp/ptp_kvm_x86.c:63:9: error: implicit declaration of function 'kvm_arch_ptp_exit'; did you mean 'kvm_arch_ptp_init'? [-Werror=implicit-function-declaration]
      63 |         kvm_arch_ptp_exit();
         |         ^~~~~~~~~~~~~~~~~
         |         kvm_arch_ptp_init
   drivers/ptp/ptp_kvm_x86.c: At top level:
>> drivers/ptp/ptp_kvm_x86.c:68:6: warning: no previous prototype for 'kvm_arch_ptp_exit' [-Wmissing-prototypes]
      68 | void kvm_arch_ptp_exit(void)
         |      ^~~~~~~~~~~~~~~~~
>> drivers/ptp/ptp_kvm_x86.c:68:6: warning: conflicting types for 'kvm_arch_ptp_exit'; have 'void(void)'
   drivers/ptp/ptp_kvm_x86.c:63:9: note: previous implicit declaration of 'kvm_arch_ptp_exit' with type 'void(void)'
      63 |         kvm_arch_ptp_exit();
         |         ^~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/kvm_arch_ptp_exit +68 drivers/ptp/ptp_kvm_x86.c

    67	
  > 68	void kvm_arch_ptp_exit(void)
    69	{
    70		if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
    71			WARN_ON(set_memory_encrypted((unsigned long)clock_pair, 1));
    72			free_page((unsigned long)clock_pair);
    73			clock_pair = NULL;
    74		}
    75	}
    76
  
kernel test robot Feb. 21, 2023, 3:58 a.m. UTC | #2
Hi Jeremi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on horms-ipvs/master]
[also build test WARNING on mst-vhost/linux-next net/master net-next/master linus/master v6.2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeremi-Piotrowski/ptp-kvm-Use-decrypted-memory-in-confidential-guest-on-x86/20230220-210441
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
patch link:    https://lore.kernel.org/r/20230220130235.2603366-1-jpiotrowski%40linux.microsoft.com
patch subject: [PATCH] ptp: kvm: Use decrypted memory in confidential guest on x86
config: arm-allyesconfig (https://download.01.org/0day-ci/archive/20230221/202302211153.uvDZ9eZu-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/0dd1701fd254692af3d0ca051e092e8dcef190c4
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jeremi-Piotrowski/ptp-kvm-Use-decrypted-memory-in-confidential-guest-on-x86/20230220-210441
        git checkout 0dd1701fd254692af3d0ca051e092e8dcef190c4
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/ptp/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302211153.uvDZ9eZu-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/ptp/ptp_kvm_arm.c:25:6: warning: no previous prototype for 'kvm_arch_ptp_exit' [-Wmissing-prototypes]
      25 | void kvm_arch_ptp_exit(void)
         |      ^~~~~~~~~~~~~~~~~


vim +/kvm_arch_ptp_exit +25 drivers/ptp/ptp_kvm_arm.c

    24	
  > 25	void kvm_arch_ptp_exit(void)
    26	{
    27	}
    28
  
Jeremi Piotrowski Feb. 21, 2023, 8:22 a.m. UTC | #3
On 21/02/2023 02:08, kernel test robot wrote:
> Hi Jeremi,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on horms-ipvs/master]
> [also build test WARNING on mst-vhost/linux-next net/master net-next/master linus/master v6.2]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Jeremi-Piotrowski/ptp-kvm-Use-decrypted-memory-in-confidential-guest-on-x86/20230220-210441
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
> patch link:    https://lore.kernel.org/r/20230220130235.2603366-1-jpiotrowski%40linux.microsoft.com
> patch subject: [PATCH] ptp: kvm: Use decrypted memory in confidential guest on x86
> config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230221/202302210943.Xq84rrhU-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
> reproduce (this is a W=1 build):
>         # https://github.com/intel-lab-lkp/linux/commit/0dd1701fd254692af3d0ca051e092e8dcef190c4
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Jeremi-Piotrowski/ptp-kvm-Use-decrypted-memory-in-confidential-guest-on-x86/20230220-210441
>         git checkout 0dd1701fd254692af3d0ca051e092e8dcef190c4
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         make W=1 O=build_dir ARCH=x86_64 olddefconfig
>         make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202302210943.Xq84rrhU-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/ptp/ptp_kvm_x86.c: In function 'kvm_arch_ptp_init':
>    drivers/ptp/ptp_kvm_x86.c:63:9: error: implicit declaration of function 'kvm_arch_ptp_exit'; did you mean 'kvm_arch_ptp_init'? [-Werror=implicit-function-declaration]
>       63 |         kvm_arch_ptp_exit();
>          |         ^~~~~~~~~~~~~~~~~
>          |         kvm_arch_ptp_init
>    drivers/ptp/ptp_kvm_x86.c: At top level:
>>> drivers/ptp/ptp_kvm_x86.c:68:6: warning: no previous prototype for 'kvm_arch_ptp_exit' [-Wmissing-prototypes]
>       68 | void kvm_arch_ptp_exit(void)
>          |      ^~~~~~~~~~~~~~~~~
>>> drivers/ptp/ptp_kvm_x86.c:68:6: warning: conflicting types for 'kvm_arch_ptp_exit'; have 'void(void)'
>    drivers/ptp/ptp_kvm_x86.c:63:9: note: previous implicit declaration of 'kvm_arch_ptp_exit' with type 'void(void)'
>       63 |         kvm_arch_ptp_exit();
>          |         ^~~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors
> 
> 
> vim +/kvm_arch_ptp_exit +68 drivers/ptp/ptp_kvm_x86.c
> 
>     67	
>   > 68	void kvm_arch_ptp_exit(void)
>     69	{
>     70		if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
>     71			WARN_ON(set_memory_encrypted((unsigned long)clock_pair, 1));
>     72			free_page((unsigned long)clock_pair);
>     73			clock_pair = NULL;
>     74		}
>     75	}
>     76	
> 

My bad - forgot to include changes to include/linux/ptp_kvm.h in the commit.
Will fix in v2, but will hold off a day or two in case someone has a suggestion
on how to reduce the allocation.

Jeremi
  
kernel test robot Feb. 21, 2023, 9:17 a.m. UTC | #4
Hi Jeremi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on horms-ipvs/master]
[also build test ERROR on mst-vhost/linux-next net/master net-next/master linus/master v6.2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeremi-Piotrowski/ptp-kvm-Use-decrypted-memory-in-confidential-guest-on-x86/20230220-210441
base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
patch link:    https://lore.kernel.org/r/20230220130235.2603366-1-jpiotrowski%40linux.microsoft.com
patch subject: [PATCH] ptp: kvm: Use decrypted memory in confidential guest on x86
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230221/202302211746.oclm0ibu-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/0dd1701fd254692af3d0ca051e092e8dcef190c4
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jeremi-Piotrowski/ptp-kvm-Use-decrypted-memory-in-confidential-guest-on-x86/20230220-210441
        git checkout 0dd1701fd254692af3d0ca051e092e8dcef190c4
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302211746.oclm0ibu-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/ptp/ptp_kvm_x86.c: In function 'kvm_arch_ptp_init':
>> drivers/ptp/ptp_kvm_x86.c:63:9: error: implicit declaration of function 'kvm_arch_ptp_exit'; did you mean 'kvm_arch_ptp_init'? [-Werror=implicit-function-declaration]
      63 |         kvm_arch_ptp_exit();
         |         ^~~~~~~~~~~~~~~~~
         |         kvm_arch_ptp_init
   drivers/ptp/ptp_kvm_x86.c: At top level:
   drivers/ptp/ptp_kvm_x86.c:68:6: warning: no previous prototype for 'kvm_arch_ptp_exit' [-Wmissing-prototypes]
      68 | void kvm_arch_ptp_exit(void)
         |      ^~~~~~~~~~~~~~~~~
   drivers/ptp/ptp_kvm_x86.c:68:6: warning: conflicting types for 'kvm_arch_ptp_exit'; have 'void(void)'
   drivers/ptp/ptp_kvm_x86.c:63:9: note: previous implicit declaration of 'kvm_arch_ptp_exit' with type 'void(void)'
      63 |         kvm_arch_ptp_exit();
         |         ^~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   drivers/ptp/ptp_kvm_common.c: In function 'ptp_kvm_exit':
>> drivers/ptp/ptp_kvm_common.c:133:9: error: implicit declaration of function 'kvm_arch_ptp_exit'; did you mean 'kvm_arch_ptp_init'? [-Werror=implicit-function-declaration]
     133 |         kvm_arch_ptp_exit();
         |         ^~~~~~~~~~~~~~~~~
         |         kvm_arch_ptp_init
   cc1: some warnings being treated as errors


vim +63 drivers/ptp/ptp_kvm_x86.c

    22	
    23	int kvm_arch_ptp_init(void)
    24	{
    25		struct page *p;
    26		long ret;
    27	
    28		if (!kvm_para_available())
    29			return -ENODEV;
    30	
    31		if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
    32			p = alloc_page(GFP_KERNEL | __GFP_ZERO);
    33			if (!p)
    34				return -ENOMEM;
    35	
    36			clock_pair = page_address(p);
    37			ret = set_memory_decrypted((unsigned long)clock_pair, 1);
    38			if (ret) {
    39				__free_page(p);
    40				clock_pair = NULL;
    41				goto nofree;
    42			}
    43		} else {
    44			clock_pair = &clock_pair_glbl;
    45		}
    46	
    47		clock_pair_gpa = slow_virt_to_phys(clock_pair);
    48		if (!pvclock_get_pvti_cpu0_va()) {
    49			ret = -ENODEV;
    50			goto err;
    51		}
    52	
    53		ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
    54				     KVM_CLOCK_PAIRING_WALLCLOCK);
    55		if (ret == -KVM_ENOSYS) {
    56			ret = -ENODEV;
    57			goto err;
    58		}
    59	
    60		return ret;
    61	
    62	err:
  > 63		kvm_arch_ptp_exit();
    64	nofree:
    65		return ret;
    66	}
    67
  

Patch

diff --git a/drivers/ptp/ptp_kvm_arm.c b/drivers/ptp/ptp_kvm_arm.c
index b7d28c8dfb84..e68e6943167b 100644
--- a/drivers/ptp/ptp_kvm_arm.c
+++ b/drivers/ptp/ptp_kvm_arm.c
@@ -22,6 +22,10 @@  int kvm_arch_ptp_init(void)
 	return 0;
 }
 
+void kvm_arch_ptp_exit(void)
+{
+}
+
 int kvm_arch_ptp_get_clock(struct timespec64 *ts)
 {
 	return kvm_arch_ptp_get_crosststamp(NULL, ts, NULL);
diff --git a/drivers/ptp/ptp_kvm_common.c b/drivers/ptp/ptp_kvm_common.c
index 9141162c4237..2418977989be 100644
--- a/drivers/ptp/ptp_kvm_common.c
+++ b/drivers/ptp/ptp_kvm_common.c
@@ -130,6 +130,7 @@  static struct kvm_ptp_clock kvm_ptp_clock;
 static void __exit ptp_kvm_exit(void)
 {
 	ptp_clock_unregister(kvm_ptp_clock.ptp_clock);
+	kvm_arch_ptp_exit();
 }
 
 static int __init ptp_kvm_init(void)
diff --git a/drivers/ptp/ptp_kvm_x86.c b/drivers/ptp/ptp_kvm_x86.c
index 4991054a2135..902844cc1a17 100644
--- a/drivers/ptp/ptp_kvm_x86.c
+++ b/drivers/ptp/ptp_kvm_x86.c
@@ -14,27 +14,64 @@ 
 #include <uapi/linux/kvm_para.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/ptp_kvm.h>
+#include <linux/set_memory.h>
 
 static phys_addr_t clock_pair_gpa;
-static struct kvm_clock_pairing clock_pair;
+static struct kvm_clock_pairing clock_pair_glbl;
+static struct kvm_clock_pairing *clock_pair;
 
 int kvm_arch_ptp_init(void)
 {
+	struct page *p;
 	long ret;
 
 	if (!kvm_para_available())
 		return -ENODEV;
 
-	clock_pair_gpa = slow_virt_to_phys(&clock_pair);
-	if (!pvclock_get_pvti_cpu0_va())
-		return -ENODEV;
+	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
+		p = alloc_page(GFP_KERNEL | __GFP_ZERO);
+		if (!p)
+			return -ENOMEM;
+
+		clock_pair = page_address(p);
+		ret = set_memory_decrypted((unsigned long)clock_pair, 1);
+		if (ret) {
+			__free_page(p);
+			clock_pair = NULL;
+			goto nofree;
+		}
+	} else {
+		clock_pair = &clock_pair_glbl;
+	}
+
+	clock_pair_gpa = slow_virt_to_phys(clock_pair);
+	if (!pvclock_get_pvti_cpu0_va()) {
+		ret = -ENODEV;
+		goto err;
+	}
 
 	ret = kvm_hypercall2(KVM_HC_CLOCK_PAIRING, clock_pair_gpa,
 			     KVM_CLOCK_PAIRING_WALLCLOCK);
-	if (ret == -KVM_ENOSYS)
-		return -ENODEV;
+	if (ret == -KVM_ENOSYS) {
+		ret = -ENODEV;
+		goto err;
+	}
 
 	return ret;
+
+err:
+	kvm_arch_ptp_exit();
+nofree:
+	return ret;
+}
+
+void kvm_arch_ptp_exit(void)
+{
+	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
+		WARN_ON(set_memory_encrypted((unsigned long)clock_pair, 1));
+		free_page((unsigned long)clock_pair);
+		clock_pair = NULL;
+	}
 }
 
 int kvm_arch_ptp_get_clock(struct timespec64 *ts)
@@ -49,8 +86,8 @@  int kvm_arch_ptp_get_clock(struct timespec64 *ts)
 		return -EOPNOTSUPP;
 	}
 
-	ts->tv_sec = clock_pair.sec;
-	ts->tv_nsec = clock_pair.nsec;
+	ts->tv_sec = clock_pair->sec;
+	ts->tv_nsec = clock_pair->nsec;
 
 	return 0;
 }
@@ -81,9 +118,9 @@  int kvm_arch_ptp_get_crosststamp(u64 *cycle, struct timespec64 *tspec,
 			pr_err_ratelimited("clock pairing hypercall ret %lu\n", ret);
 			return -EOPNOTSUPP;
 		}
-		tspec->tv_sec = clock_pair.sec;
-		tspec->tv_nsec = clock_pair.nsec;
-		*cycle = __pvclock_read_cycles(src, clock_pair.tsc);
+		tspec->tv_sec = clock_pair->sec;
+		tspec->tv_nsec = clock_pair->nsec;
+		*cycle = __pvclock_read_cycles(src, clock_pair->tsc);
 	} while (pvclock_read_retry(src, version));
 
 	*cs = &kvm_clock;