xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/13] viridian: add support for ExProcessorMasks...
@ 2020-11-24 19:07 Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
                   ` (13 more replies)
  0 siblings, 14 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel; +Cc: Paul Durrant

From: Paul Durrant <pdurrant@amazon.com>

... plus one miscellaneous cleanup patch after introducing sizeof_field().

Paul Durrant (13):
  viridian: don't blindly write to 32-bit registers is 'mode' is invalid
  viridian: move flush hypercall implementation into separate function
  viridian: move IPI hypercall implementation into separate function
  viridian: introduce a per-cpu hypercall_vpmask and accessor
    functions...
  viridian: use hypercall_vpmask in hvcall_ipi()
  viridian: use softirq batching in hvcall_ipi()
  xen/include: import sizeof_field() macro from Linux stddef.h
  viridian: add ExProcessorMasks variants of the flush hypercalls
  viridian: add ExProcessorMasks variant of the IPI hypercall
  viridian: log initial invocation of each type of hypercall
  viridian: add a new '_HVMPV_ex_processor_masks' bit into
    HVM_PARAM_VIRIDIAN...
  xl / libxl: add 'ex_processor_mask' into
    'libxl_viridian_enlightenment'
  x86: replace open-coded occurrences of sizeof_field()...

 docs/man/xl.cfg.5.pod.in             |   8 +
 tools/include/libxl.h                |   7 +
 tools/libs/light/libxl_types.idl     |   1 +
 tools/libs/light/libxl_x86.c         |   3 +
 xen/arch/x86/cpu/vpmu_intel.c        |   4 +-
 xen/arch/x86/hvm/viridian/viridian.c | 601 +++++++++++++++++++++------
 xen/arch/x86/setup.c                 |  16 +-
 xen/include/asm-x86/hvm/viridian.h   |  10 +
 xen/include/public/hvm/params.h      |   7 +-
 xen/include/xen/compiler.h           |   8 +
 10 files changed, 532 insertions(+), 133 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode' is invalid
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-25  7:51   ` Jan Beulich
  2020-11-24 19:07 ` [PATCH v3 02/13] viridian: move flush hypercall implementation into separate function Paul Durrant
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

If hvm_guest_x86_mode() returns something other than 8 or 4 then
viridian_hypercall() will return immediately but, on the way out, will write
back status as if 'mode' was 4. This patch simply makes it leave the registers
alone.

NOTE: The formatting of the 'out' label and the switch statement are also
      adjusted as per CODING_STYLE.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v2:
 - New in v2
---
 xen/arch/x86/hvm/viridian/viridian.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index dc7183a54627..54035f75cb1a 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -692,13 +692,14 @@ int viridian_hypercall(struct cpu_user_regs *regs)
         break;
     }
 
-out:
+ out:
     output.result = status;
     switch (mode) {
     case 8:
         regs->rax = output.raw;
         break;
-    default:
+
+    case 4:
         regs->rdx = output.raw >> 32;
         regs->rax = (uint32_t)output.raw;
         break;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 02/13] viridian: move flush hypercall implementation into separate function
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 03/13] viridian: move IPI " Paul Durrant
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

This patch moves the implementation of HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE/LIST
that is currently inline in viridian_hypercall() into a new hvcall_flush()
function.

The new function returns Xen erro values which are then dealt with
appropriately. A return value of -ERESTART translates to viridian_hypercall()
returning HVM_HCALL_preempted. Other return values translate to status codes
and viridian_hypercall() returning HVM_HCALL_completed. Currently the only
values, other than -ERESTART, returned by hvcall_flush() are 0 (indicating
success) or -EINVAL.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v3:
 - Adjust prototype of new function
---
 xen/arch/x86/hvm/viridian/viridian.c | 130 ++++++++++++++++-----------
 1 file changed, 78 insertions(+), 52 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 54035f75cb1a..7871e425cbfe 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -518,6 +518,69 @@ static bool need_flush(void *ctxt, struct vcpu *v)
     return vcpu_mask & (1ul << v->vcpu_id);
 }
 
+union hypercall_input {
+    uint64_t raw;
+    struct {
+        uint16_t call_code;
+        uint16_t fast:1;
+        uint16_t rsvd1:15;
+        uint16_t rep_count:12;
+        uint16_t rsvd2:4;
+        uint16_t rep_start:12;
+        uint16_t rsvd3:4;
+    };
+};
+
+union hypercall_output {
+    uint64_t raw;
+    struct {
+        uint16_t result;
+        uint16_t rsvd1;
+        uint32_t rep_complete:12;
+        uint32_t rsvd2:20;
+    };
+};
+
+static int hvcall_flush(const union hypercall_input *input,
+                        union hypercall_output *output,
+                        paddr_t input_params_gpa,
+                        paddr_t output_params_gpa)
+{
+    struct {
+        uint64_t address_space;
+        uint64_t flags;
+        uint64_t vcpu_mask;
+    } input_params;
+
+    /* These hypercalls should never use the fast-call convention. */
+    if ( input->fast )
+        return -EINVAL;
+
+    /* Get input parameters. */
+    if ( hvm_copy_from_guest_phys(&input_params, input_params_gpa,
+                                  sizeof(input_params)) != HVMTRANS_okay )
+        return -EINVAL;
+
+    /*
+     * It is not clear from the spec. if we are supposed to
+     * include current virtual CPU in the set or not in this case,
+     * so err on the safe side.
+     */
+    if ( input_params.flags & HV_FLUSH_ALL_PROCESSORS )
+        input_params.vcpu_mask = ~0ul;
+
+    /*
+     * A false return means that another vcpu is currently trying
+     * a similar operation, so back off.
+     */
+    if ( !paging_flush_tlb(need_flush, &input_params.vcpu_mask) )
+        return -ERESTART;
+
+    output->rep_complete = input->rep_count;
+
+    return 0;
+}
+
 int viridian_hypercall(struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
@@ -525,29 +588,8 @@ int viridian_hypercall(struct cpu_user_regs *regs)
     int mode = hvm_guest_x86_mode(curr);
     unsigned long input_params_gpa, output_params_gpa;
     uint16_t status = HV_STATUS_SUCCESS;
-
-    union hypercall_input {
-        uint64_t raw;
-        struct {
-            uint16_t call_code;
-            uint16_t fast:1;
-            uint16_t rsvd1:15;
-            uint16_t rep_count:12;
-            uint16_t rsvd2:4;
-            uint16_t rep_start:12;
-            uint16_t rsvd3:4;
-        };
-    } input;
-
-    union hypercall_output {
-        uint64_t raw;
-        struct {
-            uint16_t result;
-            uint16_t rsvd1;
-            uint32_t rep_complete:12;
-            uint32_t rsvd2:20;
-        };
-    } output = { 0 };
+    union hypercall_input input;
+    union hypercall_output output = {};
 
     ASSERT(is_viridian_domain(currd));
 
@@ -580,41 +622,25 @@ int viridian_hypercall(struct cpu_user_regs *regs)
     case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
     case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
     {
-        struct {
-            uint64_t address_space;
-            uint64_t flags;
-            uint64_t vcpu_mask;
-        } input_params;
+        int rc = hvcall_flush(&input, &output, input_params_gpa,
+                              output_params_gpa);
 
-        /* These hypercalls should never use the fast-call convention. */
-        status = HV_STATUS_INVALID_PARAMETER;
-        if ( input.fast )
+        switch ( rc )
+        {
+        case 0:
             break;
 
-        /* Get input parameters. */
-        if ( hvm_copy_from_guest_phys(&input_params, input_params_gpa,
-                                      sizeof(input_params)) !=
-             HVMTRANS_okay )
-            break;
-
-        /*
-         * It is not clear from the spec. if we are supposed to
-         * include current virtual CPU in the set or not in this case,
-         * so err on the safe side.
-         */
-        if ( input_params.flags & HV_FLUSH_ALL_PROCESSORS )
-            input_params.vcpu_mask = ~0ul;
-
-        /*
-         * A false return means that another vcpu is currently trying
-         * a similar operation, so back off.
-         */
-        if ( !paging_flush_tlb(need_flush, &input_params.vcpu_mask) )
+        case -ERESTART:
             return HVM_HCALL_preempted;
 
-        output.rep_complete = input.rep_count;
+        default:
+            ASSERT_UNREACHABLE();
+            /* Fallthrough */
+        case -EINVAL:
+            status = HV_STATUS_INVALID_PARAMETER;
+            break;
+        }
 
-        status = HV_STATUS_SUCCESS;
         break;
     }
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 03/13] viridian: move IPI hypercall implementation into separate function
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 02/13] viridian: move flush hypercall implementation into separate function Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 04/13] viridian: introduce a per-cpu hypercall_vpmask and accessor functions Paul Durrant
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

This patch moves the implementation of HVCALL_SEND_IPI that is currently
inline in viridian_hypercall() into a new hvcall_ipi() function.

The new function returns Xen errno values similarly to hvcall_flush(). Hence
the errno translation code in viridial_hypercall() is generalized, removing
the need for the local 'status' variable.

NOTE: The formatting of the switch statement at the top of
      viridial_hypercall() is also adjusted as per CODING_STYLE.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v3:
 - Adjust prototype of new function

v2:
 - Different formatting adjustment
---
 xen/arch/x86/hvm/viridian/viridian.c | 168 ++++++++++++++-------------
 1 file changed, 87 insertions(+), 81 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 7871e425cbfe..8ac1b32565a8 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -581,13 +581,72 @@ static int hvcall_flush(const union hypercall_input *input,
     return 0;
 }
 
+static int hvcall_ipi(const union hypercall_input *input,
+                      union hypercall_output *output,
+                      paddr_t input_params_gpa,
+                      paddr_t output_params_gpa)
+{
+    struct domain *currd = current->domain;
+    struct vcpu *v;
+    uint32_t vector;
+    uint64_t vcpu_mask;
+
+    /* Get input parameters. */
+    if ( input->fast )
+    {
+        if ( input_params_gpa >> 32 )
+            return -EINVAL;
+
+        vector = input_params_gpa;
+        vcpu_mask = output_params_gpa;
+    }
+    else
+    {
+        struct {
+            uint32_t vector;
+            uint8_t target_vtl;
+            uint8_t reserved_zero[3];
+            uint64_t vcpu_mask;
+        } input_params;
+
+        if ( hvm_copy_from_guest_phys(&input_params, input_params_gpa,
+                                      sizeof(input_params)) != HVMTRANS_okay )
+            return -EINVAL;
+
+        if ( input_params.target_vtl ||
+             input_params.reserved_zero[0] ||
+             input_params.reserved_zero[1] ||
+             input_params.reserved_zero[2] )
+            return -EINVAL;
+
+        vector = input_params.vector;
+        vcpu_mask = input_params.vcpu_mask;
+    }
+
+    if ( vector < 0x10 || vector > 0xff )
+        return -EINVAL;
+
+    for_each_vcpu ( currd, v )
+    {
+        if ( v->vcpu_id >= (sizeof(vcpu_mask) * 8) )
+            return -EINVAL;
+
+        if ( !(vcpu_mask & (1ul << v->vcpu_id)) )
+            continue;
+
+        vlapic_set_irq(vcpu_vlapic(v), vector, 0);
+    }
+
+    return 0;
+}
+
 int viridian_hypercall(struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
     struct domain *currd = curr->domain;
     int mode = hvm_guest_x86_mode(curr);
     unsigned long input_params_gpa, output_params_gpa;
-    uint16_t status = HV_STATUS_SUCCESS;
+    int rc = 0;
     union hypercall_input input;
     union hypercall_output output = {};
 
@@ -600,11 +659,13 @@ int viridian_hypercall(struct cpu_user_regs *regs)
         input_params_gpa = regs->rdx;
         output_params_gpa = regs->r8;
         break;
+
     case 4:
         input.raw = (regs->rdx << 32) | regs->eax;
         input_params_gpa = (regs->rbx << 32) | regs->ecx;
         output_params_gpa = (regs->rdi << 32) | regs->esi;
         break;
+
     default:
         goto out;
     }
@@ -616,92 +677,18 @@ int viridian_hypercall(struct cpu_user_regs *regs)
          * See section 14.5.1 of the specification.
          */
         do_sched_op(SCHEDOP_yield, guest_handle_from_ptr(NULL, void));
-        status = HV_STATUS_SUCCESS;
         break;
 
     case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
     case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
-    {
-        int rc = hvcall_flush(&input, &output, input_params_gpa,
-                              output_params_gpa);
-
-        switch ( rc )
-        {
-        case 0:
-            break;
-
-        case -ERESTART:
-            return HVM_HCALL_preempted;
-
-        default:
-            ASSERT_UNREACHABLE();
-            /* Fallthrough */
-        case -EINVAL:
-            status = HV_STATUS_INVALID_PARAMETER;
-            break;
-        }
-
+        rc = hvcall_flush(&input, &output, input_params_gpa,
+                          output_params_gpa);
         break;
-    }
 
     case HVCALL_SEND_IPI:
-    {
-        struct vcpu *v;
-        uint32_t vector;
-        uint64_t vcpu_mask;
-
-        status = HV_STATUS_INVALID_PARAMETER;
-
-        /* Get input parameters. */
-        if ( input.fast )
-        {
-            if ( input_params_gpa >> 32 )
-                break;
-
-            vector = input_params_gpa;
-            vcpu_mask = output_params_gpa;
-        }
-        else
-        {
-            struct {
-                uint32_t vector;
-                uint8_t target_vtl;
-                uint8_t reserved_zero[3];
-                uint64_t vcpu_mask;
-            } input_params;
-
-            if ( hvm_copy_from_guest_phys(&input_params, input_params_gpa,
-                                          sizeof(input_params)) !=
-                 HVMTRANS_okay )
-                break;
-
-            if ( input_params.target_vtl ||
-                 input_params.reserved_zero[0] ||
-                 input_params.reserved_zero[1] ||
-                 input_params.reserved_zero[2] )
-                break;
-
-            vector = input_params.vector;
-            vcpu_mask = input_params.vcpu_mask;
-        }
-
-        if ( vector < 0x10 || vector > 0xff )
-            break;
-
-        for_each_vcpu ( currd, v )
-        {
-            if ( v->vcpu_id >= (sizeof(vcpu_mask) * 8) )
-                break;
-
-            if ( !(vcpu_mask & (1ul << v->vcpu_id)) )
-                continue;
-
-            vlapic_set_irq(vcpu_vlapic(v), vector, 0);
-        }
-
-        status = HV_STATUS_SUCCESS;
+        rc = hvcall_ipi(&input, &output, input_params_gpa,
+                        output_params_gpa);
         break;
-    }
 
     default:
         gprintk(XENLOG_WARNING, "unimplemented hypercall %04x\n",
@@ -714,12 +701,31 @@ int viridian_hypercall(struct cpu_user_regs *regs)
          * Given that return a status of 'invalid code' has not so far
          * caused any problems it's not worth logging.
          */
-        status = HV_STATUS_INVALID_HYPERCALL_CODE;
+        rc = -EOPNOTSUPP;
         break;
     }
 
  out:
-    output.result = status;
+    switch ( rc )
+    {
+    case 0:
+        break;
+
+    case -ERESTART:
+        return HVM_HCALL_preempted;
+
+    case -EOPNOTSUPP:
+        output.result = HV_STATUS_INVALID_HYPERCALL_CODE;
+        break;
+
+    default:
+        ASSERT_UNREACHABLE();
+        /* Fallthrough */
+    case -EINVAL:
+        output.result = HV_STATUS_INVALID_PARAMETER;
+        break;
+    }
+
     switch (mode) {
     case 8:
         regs->rax = output.raw;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 04/13] viridian: introduce a per-cpu hypercall_vpmask and accessor functions...
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (2 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 03/13] viridian: move IPI " Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 05/13] viridian: use hypercall_vpmask in hvcall_ipi() Paul Durrant
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

... and make use of them in hvcall_flush()/need_flush().

Subsequent patches will need to deal with virtual processor masks potentially
wider than 64 bits. Thus, to avoid using too much stack, this patch
introduces global per-cpu virtual processor masks and converts the
implementation of hvcall_flush() to use them.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v2:
 - Modified vpmask_set() to take a base 'vp' and a 64-bit 'mask', still
   looping over the mask as bitmap.h does not provide a primitive for copying
   one mask into another at an offset
 - Added ASSERTions to verify that we don't attempt to set or test bits
   beyond the limit of the map
---
 xen/arch/x86/hvm/viridian/viridian.c | 58 ++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 8ac1b32565a8..d8f47b8528a2 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -507,15 +507,59 @@ void viridian_domain_deinit(struct domain *d)
     XFREE(d->arch.hvm.viridian);
 }
 
+struct hypercall_vpmask {
+    DECLARE_BITMAP(mask, HVM_MAX_VCPUS);
+};
+
+static DEFINE_PER_CPU(struct hypercall_vpmask, hypercall_vpmask);
+
+static void vpmask_empty(struct hypercall_vpmask *vpmask)
+{
+    bitmap_zero(vpmask->mask, HVM_MAX_VCPUS);
+}
+
+static void vpmask_set(struct hypercall_vpmask *vpmask, unsigned int vp,
+                       uint64_t mask)
+{
+    unsigned int count = sizeof(mask) * 8;
+
+    while ( count-- )
+    {
+        if ( !mask )
+            break;
+
+        if ( mask & 1 )
+        {
+            ASSERT(vp < HVM_MAX_VCPUS);
+            __set_bit(vp, vpmask->mask);
+        }
+
+        mask >>= 1;
+        vp++;
+    }
+}
+
+static void vpmask_fill(struct hypercall_vpmask *vpmask)
+{
+    bitmap_fill(vpmask->mask, HVM_MAX_VCPUS);
+}
+
+static bool vpmask_test(const struct hypercall_vpmask *vpmask,
+                        unsigned int vp)
+{
+    ASSERT(vp < HVM_MAX_VCPUS);
+    return test_bit(vp, vpmask->mask);
+}
+
 /*
  * Windows should not issue the hypercalls requiring this callback in the
  * case where vcpu_id would exceed the size of the mask.
  */
 static bool need_flush(void *ctxt, struct vcpu *v)
 {
-    uint64_t vcpu_mask = *(uint64_t *)ctxt;
+    struct hypercall_vpmask *vpmask = ctxt;
 
-    return vcpu_mask & (1ul << v->vcpu_id);
+    return vpmask_test(vpmask, v->vcpu_id);
 }
 
 union hypercall_input {
@@ -546,6 +590,7 @@ static int hvcall_flush(const union hypercall_input *input,
                         paddr_t input_params_gpa,
                         paddr_t output_params_gpa)
 {
+    struct hypercall_vpmask *vpmask = &this_cpu(hypercall_vpmask);
     struct {
         uint64_t address_space;
         uint64_t flags;
@@ -567,13 +612,18 @@ static int hvcall_flush(const union hypercall_input *input,
      * so err on the safe side.
      */
     if ( input_params.flags & HV_FLUSH_ALL_PROCESSORS )
-        input_params.vcpu_mask = ~0ul;
+        vpmask_fill(vpmask);
+    else
+    {
+        vpmask_empty(vpmask);
+        vpmask_set(vpmask, 0, input_params.vcpu_mask);
+    }
 
     /*
      * A false return means that another vcpu is currently trying
      * a similar operation, so back off.
      */
-    if ( !paging_flush_tlb(need_flush, &input_params.vcpu_mask) )
+    if ( !paging_flush_tlb(need_flush, vpmask) )
         return -ERESTART;
 
     output->rep_complete = input->rep_count;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 05/13] viridian: use hypercall_vpmask in hvcall_ipi()
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (3 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 04/13] viridian: introduce a per-cpu hypercall_vpmask and accessor functions Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 06/13] viridian: use softirq batching " Paul Durrant
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

A subsequent patch will need to IPI a mask of virtual processors potentially
wider than 64 bits. A previous patch introduced per-cpu hypercall_vpmask
to allow hvcall_flush() to deal with such wide masks. This patch modifies
the implementation of hvcall_ipi() to make use of the same mask structures,
introducing a for_each_vp() macro to facilitate traversing a mask.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v3:
 - Couple of extra 'const' qualifiers

v2:
 - Drop the 'vp' loop now that vpmask_set() will do it internally
---
 xen/arch/x86/hvm/viridian/viridian.c | 44 +++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index d8f47b8528a2..cdfeaec8b96b 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -551,6 +551,26 @@ static bool vpmask_test(const struct hypercall_vpmask *vpmask,
     return test_bit(vp, vpmask->mask);
 }
 
+static unsigned int vpmask_first(const struct hypercall_vpmask *vpmask)
+{
+    return find_first_bit(vpmask->mask, HVM_MAX_VCPUS);
+}
+
+static unsigned int vpmask_next(const struct hypercall_vpmask *vpmask,
+                                unsigned int vp)
+{
+    /*
+     * If vp + 1 > HVM_MAX_VCPUS then find_next_bit() will return
+     * HVM_MAX_VCPUS, ensuring the for_each_vp ( ... ) loop terminates.
+     */
+    return find_next_bit(vpmask->mask, HVM_MAX_VCPUS, vp + 1);
+}
+
+#define for_each_vp(vpmask, vp) \
+	for ( (vp) = vpmask_first(vpmask); \
+	      (vp) < HVM_MAX_VCPUS; \
+	      (vp) = vpmask_next(vpmask, vp) )
+
 /*
  * Windows should not issue the hypercalls requiring this callback in the
  * case where vcpu_id would exceed the size of the mask.
@@ -631,13 +651,21 @@ static int hvcall_flush(const union hypercall_input *input,
     return 0;
 }
 
+static void send_ipi(struct hypercall_vpmask *vpmask, uint8_t vector)
+{
+    struct domain *currd = current->domain;
+    unsigned int vp;
+
+    for_each_vp ( vpmask, vp )
+        vlapic_set_irq(vcpu_vlapic(currd->vcpu[vp]), vector, 0);
+}
+
 static int hvcall_ipi(const union hypercall_input *input,
                       union hypercall_output *output,
                       paddr_t input_params_gpa,
                       paddr_t output_params_gpa)
 {
-    struct domain *currd = current->domain;
-    struct vcpu *v;
+    struct hypercall_vpmask *vpmask = &this_cpu(hypercall_vpmask);
     uint32_t vector;
     uint64_t vcpu_mask;
 
@@ -676,16 +704,10 @@ static int hvcall_ipi(const union hypercall_input *input,
     if ( vector < 0x10 || vector > 0xff )
         return -EINVAL;
 
-    for_each_vcpu ( currd, v )
-    {
-        if ( v->vcpu_id >= (sizeof(vcpu_mask) * 8) )
-            return -EINVAL;
+    vpmask_empty(vpmask);
+    vpmask_set(vpmask, 0, vcpu_mask);
 
-        if ( !(vcpu_mask & (1ul << v->vcpu_id)) )
-            continue;
-
-        vlapic_set_irq(vcpu_vlapic(v), vector, 0);
-    }
+    send_ipi(vpmask, vector);
 
     return 0;
 }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 06/13] viridian: use softirq batching in hvcall_ipi()
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (4 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 05/13] viridian: use hypercall_vpmask in hvcall_ipi() Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 07/13] xen/include: import sizeof_field() macro from Linux stddef.h Paul Durrant
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Jan Beulich, Wei Liu, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

vlapic_ipi() uses a softirq batching mechanism to improve the efficiency of
sending a IPIs to large number of processors. This patch modifies send_ipi()
(the worker function called by hvcall_ipi()) to also make use of the
mechanism when there multiple bits set the hypercall_vpmask.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v2:
 - Don't add the 'nr' field to struct hypercall_vpmask and use
   bitmap_weight() instead
---
 xen/arch/x86/hvm/viridian/viridian.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index cdfeaec8b96b..4867a1bd140b 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -11,6 +11,7 @@
 #include <xen/hypercall.h>
 #include <xen/domain_page.h>
 #include <xen/param.h>
+#include <xen/softirq.h>
 #include <asm/guest/hyperv-tlfs.h>
 #include <asm/paging.h>
 #include <asm/p2m.h>
@@ -571,6 +572,11 @@ static unsigned int vpmask_next(const struct hypercall_vpmask *vpmask,
 	      (vp) < HVM_MAX_VCPUS; \
 	      (vp) = vpmask_next(vpmask, vp) )
 
+static unsigned int vpmask_nr(const struct hypercall_vpmask *vpmask)
+{
+    return bitmap_weight(vpmask->mask, HVM_MAX_VCPUS);
+}
+
 /*
  * Windows should not issue the hypercalls requiring this callback in the
  * case where vcpu_id would exceed the size of the mask.
@@ -654,10 +660,17 @@ static int hvcall_flush(const union hypercall_input *input,
 static void send_ipi(struct hypercall_vpmask *vpmask, uint8_t vector)
 {
     struct domain *currd = current->domain;
+    unsigned int nr = vpmask_nr(vpmask);
     unsigned int vp;
 
+    if ( nr > 1 )
+        cpu_raise_softirq_batch_begin();
+
     for_each_vp ( vpmask, vp )
         vlapic_set_irq(vcpu_vlapic(currd->vcpu[vp]), vector, 0);
+
+    if ( nr > 1 )
+        cpu_raise_softirq_batch_finish();
 }
 
 static int hvcall_ipi(const union hypercall_input *input,
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 07/13] xen/include: import sizeof_field() macro from Linux stddef.h
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (5 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 06/13] viridian: use softirq batching " Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 08/13] viridian: add ExProcessorMasks variants of the flush hypercalls Paul Durrant
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Jan Beulich, Andrew Cooper, George Dunlap,
	Ian Jackson, Julien Grall, Stefano Stabellini, Wei Liu

From: Paul Durrant <pdurrant@amazon.com>

Co-locate it with the definition of offsetof() (since this is also in stddef.h
in the Linux kernel source). This macro will be needed in a subsequent patch.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Julien Grall <julien@xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Wei Liu <wl@xen.org>
---
 xen/include/xen/compiler.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index c0e0ee9f27be..676c6ea1b0a0 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -76,6 +76,14 @@
 
 #define offsetof(a,b) __builtin_offsetof(a,b)
 
+/**
+ * sizeof_field(TYPE, MEMBER)
+ *
+ * @TYPE: The structure containing the field of interest
+ * @MEMBER: The field to return the size of
+ */
+#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
+
 #if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L
 #define alignof __alignof__
 #endif
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 08/13] viridian: add ExProcessorMasks variants of the flush hypercalls
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (6 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 07/13] xen/include: import sizeof_field() macro from Linux stddef.h Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 09/13] viridian: add ExProcessorMasks variant of the IPI hypercall Paul Durrant
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

The Microsoft Hypervisor TLFS specifies variants of the already implemented
HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE/LIST hypercalls that take a 'Virtual
Processor Set' as an argument rather than a simple 64-bit mask.

This patch adds a new hvcall_flush_ex() function to implement these
(HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE/LIST_EX) hypercalls. This makes use of
new helper functions, hv_vpset_nr_banks() and hv_vpset_to_vpmask(), to
determine the size of the Virtual Processor Set (so it can be copied from
guest memory) and parse it into hypercall_vpmask (respectively).

NOTE: A guest should not yet issue these hypercalls as 'ExProcessorMasks'
      support needs to be advertised via CPUID. This will be done in a
      subsequent patch.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v3:
 - Adjust one of the helper macros
 - A few more consts and type tweaks
 - Adjust prototype of new function

v2:
 - Add helper macros to define mask and struct sizes
 - Use a union to determine the size of 'hypercall_vpset'
 - Use hweight64() in hv_vpset_nr_banks()
 - Sanity check size before hvm_copy_from_guest_phys()
---
 xen/arch/x86/hvm/viridian/viridian.c | 141 +++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 4867a1bd140b..5d0b49012360 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -577,6 +577,69 @@ static unsigned int vpmask_nr(const struct hypercall_vpmask *vpmask)
     return bitmap_weight(vpmask->mask, HVM_MAX_VCPUS);
 }
 
+#define HV_VPSET_BANK_SIZE \
+    sizeof_field(struct hv_vpset, bank_contents[0])
+
+#define HV_VPSET_SIZE(banks)   \
+    (offsetof(struct hv_vpset, bank_contents) + \
+     ((banks) * HV_VPSET_BANK_SIZE))
+
+#define HV_VPSET_MAX_BANKS \
+    (sizeof_field(struct hv_vpset, valid_bank_mask) * 8)
+
+union hypercall_vpset {
+    struct hv_vpset set;
+    uint8_t pad[HV_VPSET_SIZE(HV_VPSET_MAX_BANKS)];
+};
+
+static DEFINE_PER_CPU(union hypercall_vpset, hypercall_vpset);
+
+static unsigned int hv_vpset_nr_banks(struct hv_vpset *vpset)
+{
+    return hweight64(vpset->valid_bank_mask);
+}
+
+static uint16_t hv_vpset_to_vpmask(const struct hv_vpset *set,
+                                   struct hypercall_vpmask *vpmask)
+{
+#define NR_VPS_PER_BANK (HV_VPSET_BANK_SIZE * 8)
+
+    switch ( set->format )
+    {
+    case HV_GENERIC_SET_ALL:
+        vpmask_fill(vpmask);
+        return 0;
+
+    case HV_GENERIC_SET_SPARSE_4K:
+    {
+        uint64_t bank_mask;
+        unsigned int vp, bank = 0;
+
+        vpmask_empty(vpmask);
+        for ( vp = 0, bank_mask = set->valid_bank_mask;
+              bank_mask;
+              vp += NR_VPS_PER_BANK, bank_mask >>= 1 )
+        {
+            if ( bank_mask & 1 )
+            {
+                uint64_t mask = set->bank_contents[bank];
+
+                vpmask_set(vpmask, vp, mask);
+                bank++;
+            }
+        }
+        return 0;
+    }
+
+    default:
+        break;
+    }
+
+    return -EINVAL;
+
+#undef NR_VPS_PER_BANK
+}
+
 /*
  * Windows should not issue the hypercalls requiring this callback in the
  * case where vcpu_id would exceed the size of the mask.
@@ -657,6 +720,78 @@ static int hvcall_flush(const union hypercall_input *input,
     return 0;
 }
 
+static int hvcall_flush_ex(const union hypercall_input *input,
+                           union hypercall_output *output,
+                           paddr_t input_params_gpa,
+                           paddr_t output_params_gpa)
+{
+    struct hypercall_vpmask *vpmask = &this_cpu(hypercall_vpmask);
+    struct {
+        uint64_t address_space;
+        uint64_t flags;
+        struct hv_vpset set;
+    } input_params;
+
+    /* These hypercalls should never use the fast-call convention. */
+    if ( input->fast )
+        return -EINVAL;
+
+    /* Get input parameters. */
+    if ( hvm_copy_from_guest_phys(&input_params, input_params_gpa,
+                                  sizeof(input_params)) != HVMTRANS_okay )
+        return -EINVAL;
+
+    if ( input_params.flags & HV_FLUSH_ALL_PROCESSORS )
+        vpmask_fill(vpmask);
+    else
+    {
+        union hypercall_vpset *vpset = &this_cpu(hypercall_vpset);
+        struct hv_vpset *set = &vpset->set;
+        size_t size;
+        int rc;
+
+        *set = input_params.set;
+        if ( set->format == HV_GENERIC_SET_SPARSE_4K )
+        {
+            unsigned long offset = offsetof(typeof(input_params),
+                                            set.bank_contents);
+
+            size = sizeof(*set->bank_contents) * hv_vpset_nr_banks(set);
+
+            if ( offsetof(typeof(*vpset), set.bank_contents[0]) + size >
+                 sizeof(*vpset) )
+            {
+                ASSERT_UNREACHABLE();
+                return -EINVAL;
+            }
+
+            if ( hvm_copy_from_guest_phys(&set->bank_contents[0],
+                                          input_params_gpa + offset,
+                                          size) != HVMTRANS_okay)
+                return -EINVAL;
+
+            size += sizeof(*set);
+        }
+        else
+            size = sizeof(*set);
+
+        rc = hv_vpset_to_vpmask(set, vpmask);
+        if ( rc )
+            return rc;
+    }
+
+    /*
+     * A false return means that another vcpu is currently trying
+     * a similar operation, so back off.
+     */
+    if ( !paging_flush_tlb(need_flush, vpmask) )
+        return -ERESTART;
+
+    output->rep_complete = input->rep_count;
+
+    return 0;
+}
+
 static void send_ipi(struct hypercall_vpmask *vpmask, uint8_t vector)
 {
     struct domain *currd = current->domain;
@@ -770,6 +905,12 @@ int viridian_hypercall(struct cpu_user_regs *regs)
                           output_params_gpa);
         break;
 
+    case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
+    case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
+        rc = hvcall_flush_ex(&input, &output, input_params_gpa,
+                             output_params_gpa);
+        break;
+
     case HVCALL_SEND_IPI:
         rc = hvcall_ipi(&input, &output, input_params_gpa,
                         output_params_gpa);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 09/13] viridian: add ExProcessorMasks variant of the IPI hypercall
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (7 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 08/13] viridian: add ExProcessorMasks variants of the flush hypercalls Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 10/13] viridian: log initial invocation of each type of hypercall Paul Durrant
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

A previous patch introduced variants of the flush hypercalls that take a
'Virtual Processor Set' as an argument rather than a simple 64-bit mask.
This patch introduces a similar variant of the HVCALL_SEND_IPI hypercall
(HVCALL_SEND_IPI_EX).

NOTE: As with HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE/LIST_EX, a guest should
      not yet issue the HVCALL_SEND_IPI_EX hypercall as support for
      'ExProcessorMasks' is not yet advertised via CPUID.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v3:
 - Adjust prototype of new function

v2:
 - Sanity check size before hvm_copy_from_guest_phys()
---
 xen/arch/x86/hvm/viridian/viridian.c | 74 ++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 5d0b49012360..93865be5797a 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -860,6 +860,75 @@ static int hvcall_ipi(const union hypercall_input *input,
     return 0;
 }
 
+static int hvcall_ipi_ex(const union hypercall_input *input,
+                         union hypercall_output *output,
+                         paddr_t input_params_gpa,
+                         paddr_t output_params_gpa)
+{
+    struct hypercall_vpmask *vpmask = &this_cpu(hypercall_vpmask);
+    struct {
+        uint32_t vector;
+        uint8_t target_vtl;
+        uint8_t reserved_zero[3];
+        struct hv_vpset set;
+    } input_params;
+    union hypercall_vpset *vpset = &this_cpu(hypercall_vpset);
+    struct hv_vpset *set = &vpset->set;
+    size_t size;
+    int rc;
+
+    /* These hypercalls should never use the fast-call convention. */
+    if ( input->fast )
+        return -EINVAL;
+
+    /* Get input parameters. */
+    if ( hvm_copy_from_guest_phys(&input_params, input_params_gpa,
+                                  sizeof(input_params)) != HVMTRANS_okay )
+        return -EINVAL;
+
+    if ( input_params.target_vtl ||
+         input_params.reserved_zero[0] ||
+         input_params.reserved_zero[1] ||
+         input_params.reserved_zero[2] )
+        return HV_STATUS_INVALID_PARAMETER;
+
+    if ( input_params.vector < 0x10 || input_params.vector > 0xff )
+        return HV_STATUS_INVALID_PARAMETER;
+
+    *set = input_params.set;
+    if ( set->format == HV_GENERIC_SET_SPARSE_4K )
+    {
+        unsigned long offset = offsetof(typeof(input_params),
+                                        set.bank_contents);
+
+        size = sizeof(*set->bank_contents) * hv_vpset_nr_banks(set);
+
+        if ( offsetof(typeof(*vpset), set.bank_contents[0]) + size >
+             sizeof(*vpset) )
+        {
+            ASSERT_UNREACHABLE();
+            return -EINVAL;
+        }
+
+        if ( hvm_copy_from_guest_phys(&set->bank_contents,
+                                      input_params_gpa + offset,
+                                      size) != HVMTRANS_okay)
+            return -EINVAL;
+
+        size += sizeof(*set);
+    }
+    else
+        size = sizeof(*set);
+
+    rc = hv_vpset_to_vpmask(set, vpmask);
+    if ( rc )
+        return rc;
+
+    send_ipi(vpmask, input_params.vector);
+
+    return 0;
+}
+
 int viridian_hypercall(struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
@@ -916,6 +985,11 @@ int viridian_hypercall(struct cpu_user_regs *regs)
                         output_params_gpa);
         break;
 
+    case HVCALL_SEND_IPI_EX:
+        rc = hvcall_ipi_ex(&input, &output, input_params_gpa,
+                           output_params_gpa);
+        break;
+
     default:
         gprintk(XENLOG_WARNING, "unimplemented hypercall %04x\n",
                 input.call_code);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 10/13] viridian: log initial invocation of each type of hypercall
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (8 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 09/13] viridian: add ExProcessorMasks variant of the IPI hypercall Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 11/13] viridian: add a new '_HVMPV_ex_processor_masks' bit into HVM_PARAM_VIRIDIAN Paul Durrant
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper, Roger Pau Monné

From: Paul Durrant <pdurrant@amazon.com>

To make is simpler to observe which viridian hypercalls are issued by a
particular Windows guest, this patch adds a per-domain mask to track them.
Each type of hypercall causes a different bit to be set in the mask and
when the bit transitions from clear to set, a log line is emitted showing
the name of the hypercall and the domain that issued it.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v2:
 - Use DECLARE_BITMAP() for 'hypercall_flags'
 - Use an enum for _HCALL_* values
---
 xen/arch/x86/hvm/viridian/viridian.c | 21 +++++++++++++++++++++
 xen/include/asm-x86/hvm/viridian.h   | 10 ++++++++++
 2 files changed, 31 insertions(+)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 93865be5797a..3c18908b7b35 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -933,6 +933,7 @@ int viridian_hypercall(struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
     struct domain *currd = curr->domain;
+    struct viridian_domain *vd = currd->arch.hvm.viridian;
     int mode = hvm_guest_x86_mode(curr);
     unsigned long input_params_gpa, output_params_gpa;
     int rc = 0;
@@ -962,6 +963,10 @@ int viridian_hypercall(struct cpu_user_regs *regs)
     switch ( input.call_code )
     {
     case HVCALL_NOTIFY_LONG_SPIN_WAIT:
+        if ( !test_and_set_bit(_HCALL_spin_wait, vd->hypercall_flags) )
+            printk(XENLOG_G_INFO "d%d: VIRIDIAN HVCALL_NOTIFY_LONG_SPIN_WAIT\n",
+                   currd->domain_id);
+
         /*
          * See section 14.5.1 of the specification.
          */
@@ -970,22 +975,38 @@ int viridian_hypercall(struct cpu_user_regs *regs)
 
     case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
     case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
+        if ( !test_and_set_bit(_HCALL_flush, vd->hypercall_flags) )
+            printk(XENLOG_G_INFO "%pd: VIRIDIAN HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE/LIST\n",
+                   currd);
+
         rc = hvcall_flush(&input, &output, input_params_gpa,
                           output_params_gpa);
         break;
 
     case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
     case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
+        if ( !test_and_set_bit(_HCALL_flush_ex, vd->hypercall_flags) )
+            printk(XENLOG_G_INFO "%pd: VIRIDIAN HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE/LIST_EX\n",
+                   currd);
+
         rc = hvcall_flush_ex(&input, &output, input_params_gpa,
                              output_params_gpa);
         break;
 
     case HVCALL_SEND_IPI:
+        if ( !test_and_set_bit(_HCALL_ipi, vd->hypercall_flags) )
+            printk(XENLOG_G_INFO "%pd: VIRIDIAN HVCALL_SEND_IPI\n",
+                   currd);
+
         rc = hvcall_ipi(&input, &output, input_params_gpa,
                         output_params_gpa);
         break;
 
     case HVCALL_SEND_IPI_EX:
+        if ( !test_and_set_bit(_HCALL_ipi_ex, vd->hypercall_flags) )
+            printk(XENLOG_G_INFO "%pd: VIRIDIAN HVCALL_SEND_IPI_EX\n",
+                   currd);
+
         rc = hvcall_ipi_ex(&input, &output, input_params_gpa,
                            output_params_gpa);
         break;
diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-x86/hvm/viridian.h
index cbf77d9c760b..4c8ff6e80b6f 100644
--- a/xen/include/asm-x86/hvm/viridian.h
+++ b/xen/include/asm-x86/hvm/viridian.h
@@ -55,10 +55,20 @@ struct viridian_time_ref_count
     int64_t off;
 };
 
+enum {
+    _HCALL_spin_wait,
+    _HCALL_flush,
+    _HCALL_flush_ex,
+    _HCALL_ipi,
+    _HCALL_ipi_ex,
+    _HCALL_nr /* must be last */
+};
+
 struct viridian_domain
 {
     union hv_guest_os_id guest_os_id;
     union hv_vp_assist_page_msr hypercall_gpa;
+    DECLARE_BITMAP(hypercall_flags, _HCALL_nr);
     struct viridian_time_ref_count time_ref_count;
     struct viridian_page reference_tsc;
 };
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 11/13] viridian: add a new '_HVMPV_ex_processor_masks' bit into HVM_PARAM_VIRIDIAN...
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (9 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 10/13] viridian: log initial invocation of each type of hypercall Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 12/13] xl / libxl: add 'ex_processor_mask' into 'libxl_viridian_enlightenment' Paul Durrant
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Wei Liu, Jan Beulich, Andrew Cooper,
	Roger Pau Monné,
	George Dunlap, Ian Jackson, Julien Grall, Stefano Stabellini

From: Paul Durrant <pdurrant@amazon.com>

... and advertise ExProcessorMasks support if it is set.

Support is advertised by setting bit 11 in CPUID:40000004:EAX.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Julien Grall <julien@xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/x86/hvm/viridian/viridian.c | 3 +++
 xen/include/public/hvm/params.h      | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 3c18908b7b35..32bf58db3a73 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -84,6 +84,7 @@ typedef union _HV_CRASH_CTL_REG_CONTENTS
 #define CPUID4A_MSR_BASED_APIC         (1 << 3)
 #define CPUID4A_RELAX_TIMER_INT        (1 << 5)
 #define CPUID4A_SYNTHETIC_CLUSTER_IPI  (1 << 10)
+#define CPUID4A_EX_PROCESSOR_MASKS     (1 << 11)
 
 /* Viridian CPUID leaf 6: Implementation HW features detected and in use */
 #define CPUID6A_APIC_OVERLAY    (1 << 0)
@@ -197,6 +198,8 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
             res->a |= CPUID4A_MSR_BASED_APIC;
         if ( viridian_feature_mask(d) & HVMPV_hcall_ipi )
             res->a |= CPUID4A_SYNTHETIC_CLUSTER_IPI;
+        if ( viridian_feature_mask(d) & HVMPV_ex_processor_masks )
+            res->a |= CPUID4A_EX_PROCESSOR_MASKS;
 
         /*
          * This value is the recommended number of attempts to try to
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 0e3fdca09646..3b0a0f45da53 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -164,6 +164,10 @@
 #define _HVMPV_hcall_ipi 9
 #define HVMPV_hcall_ipi (1 << _HVMPV_hcall_ipi)
 
+/* Enable ExProcessorMasks */
+#define _HVMPV_ex_processor_masks 10
+#define HVMPV_ex_processor_masks (1 << _HVMPV_ex_processor_masks)
+
 #define HVMPV_feature_mask \
         (HVMPV_base_freq | \
          HVMPV_no_freq | \
@@ -174,7 +178,8 @@
          HVMPV_crash_ctl | \
          HVMPV_synic | \
          HVMPV_stimer | \
-         HVMPV_hcall_ipi)
+         HVMPV_hcall_ipi | \
+         HVMPV_ex_processor_masks)
 
 #endif
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 12/13] xl / libxl: add 'ex_processor_mask' into 'libxl_viridian_enlightenment'
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (10 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 11/13] viridian: add a new '_HVMPV_ex_processor_masks' bit into HVM_PARAM_VIRIDIAN Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-24 19:07 ` [PATCH v3 13/13] x86: replace open-coded occurrences of sizeof_field() Paul Durrant
  2020-12-01 14:09 ` [EXTERNAL] [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
  13 siblings, 0 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel; +Cc: Paul Durrant, Ian Jackson, Wei Liu, Anthony PERARD

From: Paul Durrant <pdurrant@amazon.com>

Adding the new value into the enumeration makes it immediately available
to xl, so this patch adjusts the xl.cfg(5) documentation accordingly.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
---
 docs/man/xl.cfg.5.pod.in         | 8 ++++++++
 tools/include/libxl.h            | 7 +++++++
 tools/libs/light/libxl_types.idl | 1 +
 tools/libs/light/libxl_x86.c     | 3 +++
 4 files changed, 19 insertions(+)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 0532739c1fff..3f0f8de1e988 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2318,6 +2318,14 @@ This set incorporates use of a hypercall for interprocessor interrupts.
 This enlightenment may improve performance of Windows guests with multiple
 virtual CPUs.
 
+=item B<ex_processor_masks>
+
+This set enables new hypercall variants taking a variably-sized sparse
+B<Virtual Processor Set> as an argument, rather than a simple 64-bit
+mask. Hence this enlightenment must be specified for guests with more
+than 64 vCPUs if B<hcall_remote_tlb_flush> and/or B<hcall_ipi> are also
+specified.
+
 =item B<defaults>
 
 This is a special value that enables the default set of groups, which
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 1ea5b4f446e8..eaffccb30f37 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -444,6 +444,13 @@
  */
 #define LIBXL_HAVE_DISK_SAFE_REMOVE 1
 
+/*
+ * LIBXL_HAVE_VIRIDIAN_EX_PROCESSOR_MASKS indicates that the
+ * 'ex_processor_masks' value is present in the viridian enlightenment
+ * enumeration.
+ */
+#define LIBXL_HAVE_VIRIDIAN_EX_PROCESSOR_MASKS 1
+
 /*
  * libxl ABI compatibility
  *
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 9d3f05f39978..05324736b744 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -238,6 +238,7 @@ libxl_viridian_enlightenment = Enumeration("viridian_enlightenment", [
     (7, "synic"),
     (8, "stimer"),
     (9, "hcall_ipi"),
+    (10, "ex_processor_masks"),
     ])
 
 libxl_hdtype = Enumeration("hdtype", [
diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index e18274cc10e2..86d272999d67 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -366,6 +366,9 @@ static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid,
     if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_HCALL_IPI))
         mask |= HVMPV_hcall_ipi;
 
+    if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_EX_PROCESSOR_MASKS))
+        mask |= HVMPV_ex_processor_masks;
+
     if (mask != 0 &&
         xc_hvm_param_set(CTX->xch,
                          domid,
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 13/13] x86: replace open-coded occurrences of sizeof_field()...
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (11 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 12/13] xl / libxl: add 'ex_processor_mask' into 'libxl_viridian_enlightenment' Paul Durrant
@ 2020-11-24 19:07 ` Paul Durrant
  2020-11-25  7:45   ` Jan Beulich
  2020-11-30  2:48   ` Tian, Kevin
  2020-12-01 14:09 ` [EXTERNAL] [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
  13 siblings, 2 replies; 20+ messages in thread
From: Paul Durrant @ 2020-11-24 19:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Paul Durrant, Jun Nakajima, Kevin Tian, Jan Beulich,
	Andrew Cooper, Roger Pau Monné,
	Wei Liu

From: Paul Durrant <pdurrant@amazon.com>

... with macro evaluations, now that it is available.

A recent patch imported the sizeof_field() macro from Linux. This patch makes
use of it in places where the construct is currently open-coded.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Wei Liu <wl@xen.org>
---
 xen/arch/x86/cpu/vpmu_intel.c |  4 ++--
 xen/arch/x86/setup.c          | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 75aa11c6adec..6e97ce790037 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -90,8 +90,8 @@ static uint64_t __read_mostly global_ovf_ctrl_mask, global_ctrl_mask;
 static unsigned int __read_mostly regs_sz;
 /* Offset into context of the beginning of PMU register block */
 static const unsigned int regs_off =
-        sizeof(((struct xen_pmu_intel_ctxt *)0)->fixed_counters) +
-        sizeof(((struct xen_pmu_intel_ctxt *)0)->arch_counters);
+    sizeof_field(struct xen_pmu_intel_ctxt, fixed_counters) +
+    sizeof_field(struct xen_pmu_intel_ctxt, arch_counters);
 
 /*
  * QUIRK to workaround an issue on various family 6 cpus.
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 44c04e273537..30d6f375a3af 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1617,19 +1617,19 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     total_pages = nr_pages;
 
     /* Sanity check for unwanted bloat of certain hypercall structures. */
-    BUILD_BUG_ON(sizeof(((struct xen_platform_op *)0)->u) !=
-                 sizeof(((struct xen_platform_op *)0)->u.pad));
-    BUILD_BUG_ON(sizeof(((struct xen_domctl *)0)->u) !=
-                 sizeof(((struct xen_domctl *)0)->u.pad));
-    BUILD_BUG_ON(sizeof(((struct xen_sysctl *)0)->u) !=
-                 sizeof(((struct xen_sysctl *)0)->u.pad));
+    BUILD_BUG_ON(sizeof_field(struct xen_platform_op, u) !=
+                 sizeof_field(struct xen_platform_op, u.pad));
+    BUILD_BUG_ON(sizeof_field(struct xen_domctl, u) !=
+                 sizeof_field(struct xen_domctl, u.pad));
+    BUILD_BUG_ON(sizeof_field(struct xen_sysctl, u) !=
+                 sizeof_field(struct xen_sysctl, u.pad));
 
     BUILD_BUG_ON(sizeof(start_info_t) > PAGE_SIZE);
     BUILD_BUG_ON(sizeof(shared_info_t) > PAGE_SIZE);
     BUILD_BUG_ON(sizeof(struct vcpu_info) != 64);
 
-    BUILD_BUG_ON(sizeof(((struct compat_platform_op *)0)->u) !=
-                 sizeof(((struct compat_platform_op *)0)->u.pad));
+    BUILD_BUG_ON(sizeof_field(struct compat_platform_op, u) !=
+                 sizeof_field(struct compat_platform_op, u.pad));
     BUILD_BUG_ON(sizeof(start_info_compat_t) > PAGE_SIZE);
     BUILD_BUG_ON(sizeof(struct compat_vcpu_info) != 64);
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 13/13] x86: replace open-coded occurrences of sizeof_field()...
  2020-11-24 19:07 ` [PATCH v3 13/13] x86: replace open-coded occurrences of sizeof_field() Paul Durrant
@ 2020-11-25  7:45   ` Jan Beulich
  2020-11-30  2:48   ` Tian, Kevin
  1 sibling, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2020-11-25  7:45 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Paul Durrant, Jun Nakajima, Kevin Tian, Andrew Cooper,
	Roger Pau Monné,
	Wei Liu, xen-devel

On 24.11.2020 20:07, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
> 
> ... with macro evaluations, now that it is available.
> 
> A recent patch imported the sizeof_field() macro from Linux. This patch makes
> use of it in places where the construct is currently open-coded.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode' is invalid
  2020-11-24 19:07 ` [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
@ 2020-11-25  7:51   ` Jan Beulich
  2020-11-25  7:58     ` Durrant, Paul
  0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2020-11-25  7:51 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Paul Durrant, Wei Liu, Andrew Cooper, Roger Pau Monné, xen-devel

On 24.11.2020 20:07, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
> 
> If hvm_guest_x86_mode() returns something other than 8 or 4 then
> viridian_hypercall() will return immediately but, on the way out, will write
> back status as if 'mode' was 4. This patch simply makes it leave the registers
> alone.
> 
> NOTE: The formatting of the 'out' label and the switch statement are also
>       adjusted as per CODING_STYLE.

Partly only as far as the latter goes:

> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -692,13 +692,14 @@ int viridian_hypercall(struct cpu_user_regs *regs)
>          break;
>      }
>  
> -out:
> + out:
>      output.result = status;
>      switch (mode) {

This would want to be

    switch ( mode )
    {

I guess this could easily be taken care of while committing.

Jan


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode' is invalid
  2020-11-25  7:51   ` Jan Beulich
@ 2020-11-25  7:58     ` Durrant, Paul
  0 siblings, 0 replies; 20+ messages in thread
From: Durrant, Paul @ 2020-11-25  7:58 UTC (permalink / raw)
  To: Jan Beulich, Paul Durrant
  Cc: Wei Liu, Andrew Cooper, Roger Pau Monné, xen-devel

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 25 November 2020 07:52
> To: Paul Durrant <paul@xen.org>
> Cc: Durrant, Paul <pdurrant@amazon.co.uk>; Wei Liu <wl@xen.org>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; xen-devel@lists.xenproject.org
> Subject: RE: [EXTERNAL] [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode'
> is invalid
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 24.11.2020 20:07, Paul Durrant wrote:
> > From: Paul Durrant <pdurrant@amazon.com>
> >
> > If hvm_guest_x86_mode() returns something other than 8 or 4 then
> > viridian_hypercall() will return immediately but, on the way out, will write
> > back status as if 'mode' was 4. This patch simply makes it leave the registers
> > alone.
> >
> > NOTE: The formatting of the 'out' label and the switch statement are also
> >       adjusted as per CODING_STYLE.
> 
> Partly only as far as the latter goes:
> 
> > --- a/xen/arch/x86/hvm/viridian/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian/viridian.c
> > @@ -692,13 +692,14 @@ int viridian_hypercall(struct cpu_user_regs *regs)
> >          break;
> >      }
> >
> > -out:
> > + out:
> >      output.result = status;
> >      switch (mode) {
> 
> This would want to be
> 
>     switch ( mode )
>     {
> 

Oh, yes.

> I guess this could easily be taken care of while committing.

Thanks,

  Paul

> 
> Jan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v3 13/13] x86: replace open-coded occurrences of sizeof_field()...
  2020-11-24 19:07 ` [PATCH v3 13/13] x86: replace open-coded occurrences of sizeof_field() Paul Durrant
  2020-11-25  7:45   ` Jan Beulich
@ 2020-11-30  2:48   ` Tian, Kevin
  1 sibling, 0 replies; 20+ messages in thread
From: Tian, Kevin @ 2020-11-30  2:48 UTC (permalink / raw)
  To: Paul Durrant, xen-devel
  Cc: Paul Durrant, Nakajima, Jun, Jan Beulich, Cooper, Andrew,
	Roger Pau Monné,
	Wei Liu

> From: Paul Durrant <paul@xen.org>
> Sent: Wednesday, November 25, 2020 3:08 AM
> 
> From: Paul Durrant <pdurrant@amazon.com>
> 
> ... with macro evaluations, now that it is available.
> 
> A recent patch imported the sizeof_field() macro from Linux. This patch
> makes
> use of it in places where the construct is currently open-coded.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> ---
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> ---
>  xen/arch/x86/cpu/vpmu_intel.c |  4 ++--
>  xen/arch/x86/setup.c          | 16 ++++++++--------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/vpmu_intel.c
> b/xen/arch/x86/cpu/vpmu_intel.c
> index 75aa11c6adec..6e97ce790037 100644
> --- a/xen/arch/x86/cpu/vpmu_intel.c
> +++ b/xen/arch/x86/cpu/vpmu_intel.c
> @@ -90,8 +90,8 @@ static uint64_t __read_mostly global_ovf_ctrl_mask,
> global_ctrl_mask;
>  static unsigned int __read_mostly regs_sz;
>  /* Offset into context of the beginning of PMU register block */
>  static const unsigned int regs_off =
> -        sizeof(((struct xen_pmu_intel_ctxt *)0)->fixed_counters) +
> -        sizeof(((struct xen_pmu_intel_ctxt *)0)->arch_counters);
> +    sizeof_field(struct xen_pmu_intel_ctxt, fixed_counters) +
> +    sizeof_field(struct xen_pmu_intel_ctxt, arch_counters);
> 
>  /*
>   * QUIRK to workaround an issue on various family 6 cpus.
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 44c04e273537..30d6f375a3af 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1617,19 +1617,19 @@ void __init noreturn __start_xen(unsigned long
> mbi_p)
>      total_pages = nr_pages;
> 
>      /* Sanity check for unwanted bloat of certain hypercall structures. */
> -    BUILD_BUG_ON(sizeof(((struct xen_platform_op *)0)->u) !=
> -                 sizeof(((struct xen_platform_op *)0)->u.pad));
> -    BUILD_BUG_ON(sizeof(((struct xen_domctl *)0)->u) !=
> -                 sizeof(((struct xen_domctl *)0)->u.pad));
> -    BUILD_BUG_ON(sizeof(((struct xen_sysctl *)0)->u) !=
> -                 sizeof(((struct xen_sysctl *)0)->u.pad));
> +    BUILD_BUG_ON(sizeof_field(struct xen_platform_op, u) !=
> +                 sizeof_field(struct xen_platform_op, u.pad));
> +    BUILD_BUG_ON(sizeof_field(struct xen_domctl, u) !=
> +                 sizeof_field(struct xen_domctl, u.pad));
> +    BUILD_BUG_ON(sizeof_field(struct xen_sysctl, u) !=
> +                 sizeof_field(struct xen_sysctl, u.pad));
> 
>      BUILD_BUG_ON(sizeof(start_info_t) > PAGE_SIZE);
>      BUILD_BUG_ON(sizeof(shared_info_t) > PAGE_SIZE);
>      BUILD_BUG_ON(sizeof(struct vcpu_info) != 64);
> 
> -    BUILD_BUG_ON(sizeof(((struct compat_platform_op *)0)->u) !=
> -                 sizeof(((struct compat_platform_op *)0)->u.pad));
> +    BUILD_BUG_ON(sizeof_field(struct compat_platform_op, u) !=
> +                 sizeof_field(struct compat_platform_op, u.pad));
>      BUILD_BUG_ON(sizeof(start_info_compat_t) > PAGE_SIZE);
>      BUILD_BUG_ON(sizeof(struct compat_vcpu_info) != 64);
> 
> --
> 2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [EXTERNAL] [PATCH v3 00/13] viridian: add support for ExProcessorMasks...
  2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
                   ` (12 preceding siblings ...)
  2020-11-24 19:07 ` [PATCH v3 13/13] x86: replace open-coded occurrences of sizeof_field() Paul Durrant
@ 2020-12-01 14:09 ` Paul Durrant
  2020-12-04 10:36   ` Wei Liu
  13 siblings, 1 reply; 20+ messages in thread
From: Paul Durrant @ 2020-12-01 14:09 UTC (permalink / raw)
  To: 'Wei Liu'; +Cc: xen-devel

Wei,

  I'll likely send a v4 to address the style nit Jan picked up in patch #1 but the rest should be stable now. Could you have a look over it?

  Thanks,

    Paul

> -----Original Message-----
> From: Paul Durrant <paul@xen.org>
> Sent: 24 November 2020 19:08
> To: xen-devel@lists.xenproject.org
> Cc: Durrant, Paul <pdurrant@amazon.co.uk>
> Subject: [EXTERNAL] [PATCH v3 00/13] viridian: add support for ExProcessorMasks...
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> From: Paul Durrant <pdurrant@amazon.com>
> 
> ... plus one miscellaneous cleanup patch after introducing sizeof_field().
> 
> Paul Durrant (13):
>   viridian: don't blindly write to 32-bit registers is 'mode' is invalid
>   viridian: move flush hypercall implementation into separate function
>   viridian: move IPI hypercall implementation into separate function
>   viridian: introduce a per-cpu hypercall_vpmask and accessor
>     functions...
>   viridian: use hypercall_vpmask in hvcall_ipi()
>   viridian: use softirq batching in hvcall_ipi()
>   xen/include: import sizeof_field() macro from Linux stddef.h
>   viridian: add ExProcessorMasks variants of the flush hypercalls
>   viridian: add ExProcessorMasks variant of the IPI hypercall
>   viridian: log initial invocation of each type of hypercall
>   viridian: add a new '_HVMPV_ex_processor_masks' bit into
>     HVM_PARAM_VIRIDIAN...
>   xl / libxl: add 'ex_processor_mask' into
>     'libxl_viridian_enlightenment'
>   x86: replace open-coded occurrences of sizeof_field()...
> 
>  docs/man/xl.cfg.5.pod.in             |   8 +
>  tools/include/libxl.h                |   7 +
>  tools/libs/light/libxl_types.idl     |   1 +
>  tools/libs/light/libxl_x86.c         |   3 +
>  xen/arch/x86/cpu/vpmu_intel.c        |   4 +-
>  xen/arch/x86/hvm/viridian/viridian.c | 601 +++++++++++++++++++++------
>  xen/arch/x86/setup.c                 |  16 +-
>  xen/include/asm-x86/hvm/viridian.h   |  10 +
>  xen/include/public/hvm/params.h      |   7 +-
>  xen/include/xen/compiler.h           |   8 +
>  10 files changed, 532 insertions(+), 133 deletions(-)
> 
> --
> 2.20.1




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [EXTERNAL] [PATCH v3 00/13] viridian: add support for ExProcessorMasks...
  2020-12-01 14:09 ` [EXTERNAL] [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
@ 2020-12-04 10:36   ` Wei Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Wei Liu @ 2020-12-04 10:36 UTC (permalink / raw)
  To: paul; +Cc: 'Wei Liu', xen-devel

On Tue, Dec 01, 2020 at 02:09:40PM -0000, Paul Durrant wrote:
> Wei,
> 
>   I'll likely send a v4 to address the style nit Jan picked up in patch #1 but the rest should be stable now. Could you have a look over it?

I've only been able to skim-read this patch set, but I agree in general
that adding ExProcessorMasks support is a good idea. It is needed to
cope with more than 64 cpus as far as I can tell.

With Jan's comments addressed.

Acked-by: Wei Liu <wl@xen.org>

Wei.



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-12-04 10:36 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-24 19:07 [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
2020-11-24 19:07 ` [PATCH v3 01/13] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
2020-11-25  7:51   ` Jan Beulich
2020-11-25  7:58     ` Durrant, Paul
2020-11-24 19:07 ` [PATCH v3 02/13] viridian: move flush hypercall implementation into separate function Paul Durrant
2020-11-24 19:07 ` [PATCH v3 03/13] viridian: move IPI " Paul Durrant
2020-11-24 19:07 ` [PATCH v3 04/13] viridian: introduce a per-cpu hypercall_vpmask and accessor functions Paul Durrant
2020-11-24 19:07 ` [PATCH v3 05/13] viridian: use hypercall_vpmask in hvcall_ipi() Paul Durrant
2020-11-24 19:07 ` [PATCH v3 06/13] viridian: use softirq batching " Paul Durrant
2020-11-24 19:07 ` [PATCH v3 07/13] xen/include: import sizeof_field() macro from Linux stddef.h Paul Durrant
2020-11-24 19:07 ` [PATCH v3 08/13] viridian: add ExProcessorMasks variants of the flush hypercalls Paul Durrant
2020-11-24 19:07 ` [PATCH v3 09/13] viridian: add ExProcessorMasks variant of the IPI hypercall Paul Durrant
2020-11-24 19:07 ` [PATCH v3 10/13] viridian: log initial invocation of each type of hypercall Paul Durrant
2020-11-24 19:07 ` [PATCH v3 11/13] viridian: add a new '_HVMPV_ex_processor_masks' bit into HVM_PARAM_VIRIDIAN Paul Durrant
2020-11-24 19:07 ` [PATCH v3 12/13] xl / libxl: add 'ex_processor_mask' into 'libxl_viridian_enlightenment' Paul Durrant
2020-11-24 19:07 ` [PATCH v3 13/13] x86: replace open-coded occurrences of sizeof_field() Paul Durrant
2020-11-25  7:45   ` Jan Beulich
2020-11-30  2:48   ` Tian, Kevin
2020-12-01 14:09 ` [EXTERNAL] [PATCH v3 00/13] viridian: add support for ExProcessorMasks Paul Durrant
2020-12-04 10:36   ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).