[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN][PATCH v3] x86: make Viridian support optional


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
  • Date: Thu, 18 Sep 2025 20:17:44 +0300
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=9s29UzLn3S0yzMtXZih6lU9rifFrGf4nh6Jcza3sPhM=; b=XNg14fD8M/N+BQ1SaZ+BOX6/OW4sp5drf2K014A9XsYp59zT5wkcLk7b6h8P7v+VCTFAFENHtt1ad6cwMeo41TxLqyxPiwSSmDnq86kgij4Mk9ZxAo2wWusvE2OuB0g8m8fqMpSfATI1Xc2vyuLb/YWOH6XSZbY0P/zFaYPkrEBXm6oodIXvD7vCtOJ/ESj5as3Siup7NVZoXIJRj7fy4xhJx87jZCZxCrkKLBbniJ+W/FItLYWcQ0kUC01airjXJe25b41FH3iC+7A8WOZ/zZ1fYpXqH46G0DdFzRBcmMlDfTgzRpr6OOUi2UjorHHIzPkvva/JuMlwukvaawb6iA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=IVmmxeM1Mze2SbriNnJ9KiR4uhT1RjHLtRMxae6Y9vthiiW9TPldXQOTLNYR2I5h+yS/2WtSDjORyiy7/jcdfDy7QzpxE7v5N3WBu+pKjqipQwNDsJsmI5rWzoAFJTY1c0pOBVZ8eEbmFIImtQwGZRZH1KYIBlR4ZbEQNxT1WcJWUXkrawSBvDOWJzxXLPKgADwKDPEQvfPH5Q2WiNak63ihEoKFkcBHuvlQNqW0R0ha5ipw8AHJpFtZJHkO6m+mrSLhgmx3f+1Lbk0DOq6Mw75NS5XPUOw0VWIbbqT1cIar2fWEsF6ZdFlIHuwJkswt1Z8BYREHHlTlHg7aac7nkA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=epam.com;
  • Cc: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 18 Sep 2025 17:18:01 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>



On 18.09.25 18:41, Jan Beulich wrote:
On 16.09.2025 15:41, Grygorii Strashko wrote:
--- a/xen/arch/x86/hvm/Kconfig
+++ b/xen/arch/x86/hvm/Kconfig
@@ -62,6 +62,16 @@ config ALTP2M
If unsure, stay with defaults. +config HVM_VIRIDIAN

I may have said so already on v1: I'm not quite convinced of the need
or usefulness of the HVM_ part here. Viridian necessarily means HVM,
aiui.

sure.


--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -1116,14 +1116,14 @@ static int cf_check viridian_save_domain_ctxt(
  {
      const struct domain *d = v->domain;
      const struct viridian_domain *vd = d->arch.hvm.viridian;
-    struct hvm_viridian_domain_context ctxt = {
-        .hypercall_gpa = vd->hypercall_gpa.raw,
-        .guest_os_id = vd->guest_os_id.raw,
-    };
+    struct hvm_viridian_domain_context ctxt = {};
if ( !is_viridian_domain(d) )
          return 0;

This check doesn't check for vd being non-NULL, so this still feels a little
fragile, even if it looks correct now.

Hm. May be I missing smth., but
- if is_viridian_domain(d) and viridian_domain_init() succeeded
  then d->arch.hvm.viridian != NULL, like always
  (otherwise domain will not be created)

- if !is_viridian_domain() then code will not go further

so I'm missing to see how !d->arch.hvm.viridian (!vd) can happen here.

To be paranoid can also add:
 if (!vd)
    return -EINVAL;


+    ctxt.hypercall_gpa = vd->hypercall_gpa.raw;
+    ctxt.guest_os_id = vd->guest_os_id.raw,
+
      viridian_time_save_domain_ctxt(d, &ctxt);
      viridian_synic_save_domain_ctxt(d, &ctxt);

Just below here we have viridian_load_domain_ctxt(), which I'm pretty sure
now also needs to gain some check: Save records coming from user space, we
can't really rely on there being none of this type for a non-Viridian domain.

As per my understanding:
viridian_load_domain_ctxt() calls hvm_load_entry_zeroextend() which
should not succeed if context was not saved (which shouldn't happen for
!is_viridian_domain(d) case).

To be paranoid can also add in 
viridian_load_domain_ctxt/viridian_load_vcpu_ctxt:
   if ( !is_viridian_domain(d) )
        return -ENODEV;

--
Best regards,
-grygorii




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.