* [PATCH v2 00/12] viridian: add support for ExProcessorMasks
@ 2020-11-20 9:48 Paul Durrant
2020-11-20 9:48 ` [PATCH v2 01/12] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
` (11 more replies)
0 siblings, 12 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 UTC (permalink / raw)
To: xen-devel
Cc: Paul Durrant, Andrew Cooper, Anthony PERARD, George Dunlap,
Ian Jackson, Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini, Wei Liu
From: Paul Durrant <pdurrant@amazon.com>
Paul Durrant (12):
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'
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/hvm/viridian/viridian.c | 601 +++++++++++++++++++++------
xen/include/asm-x86/hvm/viridian.h | 10 +
xen/include/public/hvm/params.h | 7 +-
xen/include/xen/compiler.h | 8 +
8 files changed, 522 insertions(+), 123 deletions(-)
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Wei Liu <wl@xen.org>
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 01/12] viridian: don't blindly write to 32-bit registers is 'mode' is invalid
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 14:20 ` Jan Beulich
2020-11-20 9:48 ` [PATCH v2 02/12] viridian: move flush hypercall implementation into separate function Paul Durrant
` (10 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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] 23+ messages in thread
* [PATCH v2 02/12] viridian: move flush hypercall implementation into separate function
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
2020-11-20 9:48 ` [PATCH v2 01/12] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 9:48 ` [PATCH v2 03/12] viridian: move IPI " Paul Durrant
` (9 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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>
---
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..15b1c96fe13b 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(union hypercall_input *input,
+ union hypercall_output *output,
+ unsigned long input_params_gpa,
+ unsigned long 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] 23+ messages in thread
* [PATCH v2 03/12] viridian: move IPI hypercall implementation into separate function
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
2020-11-20 9:48 ` [PATCH v2 01/12] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
2020-11-20 9:48 ` [PATCH v2 02/12] viridian: move flush hypercall implementation into separate function Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 9:48 ` [PATCH v2 04/12] viridian: introduce a per-cpu hypercall_vpmask and accessor functions Paul Durrant
` (8 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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>
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 15b1c96fe13b..d9864cdc0b7d 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(union hypercall_input *input,
return 0;
}
+static int hvcall_ipi(union hypercall_input *input,
+ union hypercall_output *output,
+ unsigned long input_params_gpa,
+ unsigned long 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] 23+ messages in thread
* [PATCH v2 04/12] viridian: introduce a per-cpu hypercall_vpmask and accessor functions...
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (2 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 03/12] viridian: move IPI " Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 9:48 ` [PATCH v2 05/12] viridian: use hypercall_vpmask in hvcall_ipi() Paul Durrant
` (7 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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 d9864cdc0b7d..334d8527ff59 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(union hypercall_input *input,
unsigned long input_params_gpa,
unsigned long 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(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] 23+ messages in thread
* [PATCH v2 05/12] viridian: use hypercall_vpmask in hvcall_ipi()
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (3 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 04/12] viridian: introduce a per-cpu hypercall_vpmask and accessor functions Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 15:09 ` Jan Beulich
2020-11-20 9:48 ` [PATCH v2 06/12] viridian: use softirq batching " Paul Durrant
` (6 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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>
v2:
- Drop the 'vp' loop now that vpmask_set() will do it internally
---
xen/arch/x86/hvm/viridian/viridian.c | 43 +++++++++++++++++++++-------
1 file changed, 32 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 334d8527ff59..d8d8ecc89c80 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -551,6 +551,25 @@ static bool vpmask_test(const struct hypercall_vpmask *vpmask,
return test_bit(vp, vpmask->mask);
}
+static unsigned int vpmask_first(struct hypercall_vpmask *vpmask)
+{
+ return find_first_bit(vpmask->mask, HVM_MAX_VCPUS);
+}
+
+static unsigned int vpmask_next(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 +650,21 @@ static int hvcall_flush(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(union hypercall_input *input,
union hypercall_output *output,
unsigned long input_params_gpa,
unsigned long 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 +703,10 @@ static int hvcall_ipi(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] 23+ messages in thread
* [PATCH v2 06/12] viridian: use softirq batching in hvcall_ipi()
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (4 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 05/12] viridian: use hypercall_vpmask in hvcall_ipi() Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 15:11 ` Jan Beulich
2020-11-20 9:48 ` [PATCH v2 07/12] xen/include: import sizeof_field() macro from Linux stddef.h Paul Durrant
` (5 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 UTC (permalink / raw)
To: xen-devel
Cc: Paul Durrant, Wei Liu, Jan Beulich, 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>
---
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:
- 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 d8d8ecc89c80..d6f47b28c1e6 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>
@@ -570,6 +571,11 @@ static unsigned int vpmask_next(struct hypercall_vpmask *vpmask, unsigned int vp
(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.
@@ -653,10 +659,17 @@ static int hvcall_flush(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(union hypercall_input *input,
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 07/12] xen/include: import sizeof_field() macro from Linux stddef.h
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (5 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 06/12] viridian: use softirq batching " Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 15:15 ` Jan Beulich
2020-11-20 9:48 ` [PATCH v2 08/12] viridian: add ExProcessorMasks variants of the flush hypercalls Paul Durrant
` (4 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 UTC (permalink / raw)
To: xen-devel
Cc: Paul Durrant, Andrew Cooper, George Dunlap, Ian Jackson,
Jan Beulich, 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>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Jan Beulich <jbeulich@suse.com>
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] 23+ messages in thread
* [PATCH v2 08/12] viridian: add ExProcessorMasks variants of the flush hypercalls
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (6 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 07/12] xen/include: import sizeof_field() macro from Linux stddef.h Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-24 16:55 ` Jan Beulich
2020-11-20 9:48 ` [PATCH v2 09/12] viridian: add ExProcessorMasks variant of the IPI hypercall Paul Durrant
` (3 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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>
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 | 142 +++++++++++++++++++++++++++
1 file changed, 142 insertions(+)
diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index d6f47b28c1e6..e736c0739da0 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -576,6 +576,70 @@ 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) \
+ (sizeof(struct hv_vpset) + (banks * HV_VPSET_BANK_SIZE))
+
+#define HV_VPSET_MAX_BANKS \
+ (sizeof_field(struct hv_vpset, valid_bank_mask) * 8)
+
+struct hypercall_vpset {
+ union {
+ struct hv_vpset set;
+ uint8_t pad[HV_VPSET_SIZE(HV_VPSET_MAX_BANKS)];
+ };
+};
+
+static DEFINE_PER_CPU(struct 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(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.
@@ -656,6 +720,78 @@ static int hvcall_flush(union hypercall_input *input,
return 0;
}
+static int hvcall_flush_ex(union hypercall_input *input,
+ union hypercall_output *output,
+ unsigned long input_params_gpa,
+ unsigned long 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
+ {
+ struct 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;
@@ -769,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] 23+ messages in thread
* [PATCH v2 09/12] viridian: add ExProcessorMasks variant of the IPI hypercall
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (7 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 08/12] viridian: add ExProcessorMasks variants of the flush hypercalls Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-24 16:58 ` Jan Beulich
2020-11-20 9:48 ` [PATCH v2 10/12] viridian: log initial invocation of each type of hypercall Paul Durrant
` (2 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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>
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 e736c0739da0..74227fecbcd4 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(union hypercall_input *input,
return 0;
}
+static int hvcall_ipi_ex(union hypercall_input *input,
+ union hypercall_output *output,
+ unsigned long input_params_gpa,
+ unsigned long 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;
+ struct 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] 23+ messages in thread
* [PATCH v2 10/12] viridian: log initial invocation of each type of hypercall
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (8 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 09/12] viridian: add ExProcessorMasks variant of the IPI hypercall Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 9:48 ` [PATCH v2 11/12] viridian: add a new '_HVMPV_ex_processor_masks' bit into HVM_PARAM_VIRIDIAN Paul Durrant
2020-11-20 9:49 ` [PATCH v2 12/12] xl / libxl: add 'ex_processor_mask' into 'libxl_viridian_enlightenment' Paul Durrant
11 siblings, 0 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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 74227fecbcd4..de3f5f86777c 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] 23+ messages in thread
* [PATCH v2 11/12] viridian: add a new '_HVMPV_ex_processor_masks' bit into HVM_PARAM_VIRIDIAN...
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (9 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 10/12] viridian: log initial invocation of each type of hypercall Paul Durrant
@ 2020-11-20 9:48 ` Paul Durrant
2020-11-20 9:49 ` [PATCH v2 12/12] xl / libxl: add 'ex_processor_mask' into 'libxl_viridian_enlightenment' Paul Durrant
11 siblings, 0 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:48 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 de3f5f86777c..54b20188e326 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] 23+ messages in thread
* [PATCH v2 12/12] xl / libxl: add 'ex_processor_mask' into 'libxl_viridian_enlightenment'
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
` (10 preceding siblings ...)
2020-11-20 9:48 ` [PATCH v2 11/12] viridian: add a new '_HVMPV_ex_processor_masks' bit into HVM_PARAM_VIRIDIAN Paul Durrant
@ 2020-11-20 9:49 ` Paul Durrant
11 siblings, 0 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 9:49 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] 23+ messages in thread
* Re: [PATCH v2 01/12] viridian: don't blindly write to 32-bit registers is 'mode' is invalid
2020-11-20 9:48 ` [PATCH v2 01/12] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
@ 2020-11-20 14:20 ` Jan Beulich
2020-11-20 14:25 ` Durrant, Paul
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2020-11-20 14:20 UTC (permalink / raw)
To: Paul Durrant
Cc: Paul Durrant, Wei Liu, Andrew Cooper, Roger Pau Monné, xen-devel
On 20.11.2020 10:48, 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.
IOW 16-bit protected mode and real mode aren't allowed to make
hypercalls (supported also be the earlier switch() in the
function)? But then, to achieve what you want, wouldn't it be
more direct to simply "return HVM_HCALL_completed;" straight
from that earlier switch()'s default case? At which point the
switch() you modify could become if/else? Anyway - you're the
maintainer, I'm just wondering ...
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 01/12] viridian: don't blindly write to 32-bit registers is 'mode' is invalid
2020-11-20 14:20 ` Jan Beulich
@ 2020-11-20 14:25 ` Durrant, Paul
0 siblings, 0 replies; 23+ messages in thread
From: Durrant, Paul @ 2020-11-20 14:25 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: 20 November 2020 14:20
> 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 v2 01/12] 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 20.11.2020 10:48, 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.
>
> IOW 16-bit protected mode and real mode aren't allowed to make
> hypercalls (supported also be the earlier switch() in the
> function)?
I don't think running enlightened versions of OS/2 1.3 is really a use case :-)
> But then, to achieve what you want, wouldn't it be
> more direct to simply "return HVM_HCALL_completed;" straight
> from that earlier switch()'s default case? At which point the
> switch() you modify could become if/else? Anyway - you're the
> maintainer, I'm just wondering ...
>
It could be done that way but I prefer the exit path via goto.
Paul
> Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 05/12] viridian: use hypercall_vpmask in hvcall_ipi()
2020-11-20 9:48 ` [PATCH v2 05/12] viridian: use hypercall_vpmask in hvcall_ipi() Paul Durrant
@ 2020-11-20 15:09 ` Jan Beulich
2020-11-20 15:12 ` Durrant, Paul
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2020-11-20 15:09 UTC (permalink / raw)
To: Paul Durrant
Cc: Paul Durrant, Wei Liu, Andrew Cooper, Roger Pau Monné, xen-devel
On 20.11.2020 10:48, Paul Durrant wrote:
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -551,6 +551,25 @@ static bool vpmask_test(const struct hypercall_vpmask *vpmask,
> return test_bit(vp, vpmask->mask);
> }
>
> +static unsigned int vpmask_first(struct hypercall_vpmask *vpmask)
Now this and ...
> +{
> + return find_first_bit(vpmask->mask, HVM_MAX_VCPUS);
> +}
> +
> +static unsigned int vpmask_next(struct hypercall_vpmask *vpmask, unsigned int vp)
... this should really have pointers to const as parameters.
> @@ -631,13 +650,21 @@ static int hvcall_flush(union hypercall_input *input,
> return 0;
> }
>
> +static void send_ipi(struct hypercall_vpmask *vpmask, uint8_t vector)
And I guess this one should, too.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 06/12] viridian: use softirq batching in hvcall_ipi()
2020-11-20 9:48 ` [PATCH v2 06/12] viridian: use softirq batching " Paul Durrant
@ 2020-11-20 15:11 ` Jan Beulich
0 siblings, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2020-11-20 15:11 UTC (permalink / raw)
To: Paul Durrant
Cc: Paul Durrant, Wei Liu, Andrew Cooper, Roger Pau Monné, xen-devel
On 20.11.2020 10:48, Paul Durrant wrote:
> 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>
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 05/12] viridian: use hypercall_vpmask in hvcall_ipi()
2020-11-20 15:09 ` Jan Beulich
@ 2020-11-20 15:12 ` Durrant, Paul
0 siblings, 0 replies; 23+ messages in thread
From: Durrant, Paul @ 2020-11-20 15:12 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: 20 November 2020 15:09
> 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 v2 05/12] viridian: use hypercall_vpmask in hvcall_ipi()
>
> 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 20.11.2020 10:48, Paul Durrant wrote:
> > --- a/xen/arch/x86/hvm/viridian/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian/viridian.c
> > @@ -551,6 +551,25 @@ static bool vpmask_test(const struct hypercall_vpmask *vpmask,
> > return test_bit(vp, vpmask->mask);
> > }
> >
> > +static unsigned int vpmask_first(struct hypercall_vpmask *vpmask)
>
> Now this and ...
>
> > +{
> > + return find_first_bit(vpmask->mask, HVM_MAX_VCPUS);
> > +}
> > +
> > +static unsigned int vpmask_next(struct hypercall_vpmask *vpmask, unsigned int vp)
>
> ... this should really have pointers to const as parameters.
>
> > @@ -631,13 +650,21 @@ static int hvcall_flush(union hypercall_input *input,
> > return 0;
> > }
> >
> > +static void send_ipi(struct hypercall_vpmask *vpmask, uint8_t vector)
>
> And I guess this one should, too.
>
True, they can be const.
Paul
> Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 07/12] xen/include: import sizeof_field() macro from Linux stddef.h
2020-11-20 9:48 ` [PATCH v2 07/12] xen/include: import sizeof_field() macro from Linux stddef.h Paul Durrant
@ 2020-11-20 15:15 ` Jan Beulich
2020-11-20 15:24 ` Paul Durrant
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2020-11-20 15:15 UTC (permalink / raw)
To: Paul Durrant
Cc: Paul Durrant, Andrew Cooper, George Dunlap, Ian Jackson,
Julien Grall, Stefano Stabellini, Wei Liu, xen-devel
On 20.11.2020 10:48, Paul Durrant wrote:
> 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>
You don't fancy replacing in the tree what is now effectively
open-coding this construct, I guess? I'll try to remember to
do so once this has gone in...
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 07/12] xen/include: import sizeof_field() macro from Linux stddef.h
2020-11-20 15:15 ` Jan Beulich
@ 2020-11-20 15:24 ` Paul Durrant
0 siblings, 0 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-20 15:24 UTC (permalink / raw)
To: 'Jan Beulich'
Cc: 'Paul Durrant', 'Andrew Cooper',
'George Dunlap', 'Ian Jackson',
'Julien Grall', 'Stefano Stabellini',
'Wei Liu',
xen-devel
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 20 November 2020 15:16
> To: Paul Durrant <paul@xen.org>
> Cc: Paul Durrant <pdurrant@amazon.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap
> <george.dunlap@citrix.com>; Ian Jackson <iwj@xenproject.org>; Julien Grall <julien@xen.org>; Stefano
> Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v2 07/12] xen/include: import sizeof_field() macro from Linux stddef.h
>
> On 20.11.2020 10:48, Paul Durrant wrote:
> > 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>
>
> You don't fancy replacing in the tree what is now effectively
> open-coding this construct, I guess? I'll try to remember to
> do so once this has gone in...
>
I'll see what I can find after this series is in, unless you get there first :-)
Paul
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 08/12] viridian: add ExProcessorMasks variants of the flush hypercalls
2020-11-20 9:48 ` [PATCH v2 08/12] viridian: add ExProcessorMasks variants of the flush hypercalls Paul Durrant
@ 2020-11-24 16:55 ` Jan Beulich
2020-11-24 18:40 ` Paul Durrant
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2020-11-24 16:55 UTC (permalink / raw)
To: Paul Durrant
Cc: Paul Durrant, Wei Liu, Andrew Cooper, Roger Pau Monné, xen-devel
On 20.11.2020 10:48, Paul Durrant wrote:
> 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>
Just a couple of cosmetic remarks, apart from them this looks
good to me, albeit some of the size calculations are quite,
well, involved. I guess like with most parts if this series,
in the end Wei will need to give his ack.
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -576,6 +576,70 @@ 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) \
> + (sizeof(struct hv_vpset) + (banks * HV_VPSET_BANK_SIZE))
Personally I think this would be better done using
offsetof(struct hv_vpset, bank_contents), but you're the maintainer.
However, "banks" wants parenthesizing, just in case.
> +#define HV_VPSET_MAX_BANKS \
> + (sizeof_field(struct hv_vpset, valid_bank_mask) * 8)
> +
> +struct hypercall_vpset {
> + union {
> + struct hv_vpset set;
> + uint8_t pad[HV_VPSET_SIZE(HV_VPSET_MAX_BANKS)];
> + };
> +};
A struct with just a union as member could be expressed as a simple
union - any reason you prefer the slightly more involved variant?
> +static DEFINE_PER_CPU(struct 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(struct hv_vpset *set,
const?
> @@ -656,6 +720,78 @@ static int hvcall_flush(union hypercall_input *input,
> return 0;
> }
>
> +static int hvcall_flush_ex(union hypercall_input *input,
const again?
> + union hypercall_output *output,
> + unsigned long input_params_gpa,
> + unsigned long output_params_gpa)
Mainly for cosmetic reasons and to be in sync with
hvm_copy_from_guest_phys()'s respective parameter, perhaps both
would better be paddr_t?
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 09/12] viridian: add ExProcessorMasks variant of the IPI hypercall
2020-11-20 9:48 ` [PATCH v2 09/12] viridian: add ExProcessorMasks variant of the IPI hypercall Paul Durrant
@ 2020-11-24 16:58 ` Jan Beulich
0 siblings, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2020-11-24 16:58 UTC (permalink / raw)
To: Paul Durrant
Cc: Paul Durrant, Wei Liu, Andrew Cooper, Roger Pau Monné, xen-devel
On 20.11.2020 10:48, Paul Durrant wrote:
> 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>
I guess most remarks given for the previous patch apply here
as well.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 08/12] viridian: add ExProcessorMasks variants of the flush hypercalls
2020-11-24 16:55 ` Jan Beulich
@ 2020-11-24 18:40 ` Paul Durrant
0 siblings, 0 replies; 23+ messages in thread
From: Paul Durrant @ 2020-11-24 18:40 UTC (permalink / raw)
To: 'Jan Beulich'
Cc: 'Paul Durrant', 'Wei Liu',
'Andrew Cooper', 'Roger Pau Monné',
xen-devel
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 24 November 2020 16:56
> To: Paul Durrant <paul@xen.org>
> Cc: Paul Durrant <pdurrant@amazon.com>; 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: [PATCH v2 08/12] viridian: add ExProcessorMasks variants of the flush hypercalls
>
> On 20.11.2020 10:48, Paul Durrant wrote:
> > 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>
>
> Just a couple of cosmetic remarks, apart from them this looks
> good to me, albeit some of the size calculations are quite,
> well, involved. I guess like with most parts if this series,
> in the end Wei will need to give his ack.
>
> > --- a/xen/arch/x86/hvm/viridian/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian/viridian.c
> > @@ -576,6 +576,70 @@ 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) \
> > + (sizeof(struct hv_vpset) + (banks * HV_VPSET_BANK_SIZE))
>
> Personally I think this would be better done using
> offsetof(struct hv_vpset, bank_contents), but you're the maintainer.
> However, "banks" wants parenthesizing, just in case.
>
No, I actually prefer using offsetof() and yes I do indeed need to parenthesize 'banks'.
> > +#define HV_VPSET_MAX_BANKS \
> > + (sizeof_field(struct hv_vpset, valid_bank_mask) * 8)
> > +
> > +struct hypercall_vpset {
> > + union {
> > + struct hv_vpset set;
> > + uint8_t pad[HV_VPSET_SIZE(HV_VPSET_MAX_BANKS)];
> > + };
> > +};
>
> A struct with just a union as member could be expressed as a simple
> union - any reason you prefer the slightly more involved variant?
>
Not really... it's only that it was a struct in the original patch. I'll change to using a union.
> > +static DEFINE_PER_CPU(struct 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(struct hv_vpset *set,
>
> const?
>
Ok.
> > @@ -656,6 +720,78 @@ static int hvcall_flush(union hypercall_input *input,
> > return 0;
> > }
> >
> > +static int hvcall_flush_ex(union hypercall_input *input,
>
> const again?
>
True, but I'll need to go back and do that for the others too.
> > + union hypercall_output *output,
> > + unsigned long input_params_gpa,
> > + unsigned long output_params_gpa)
>
> Mainly for cosmetic reasons and to be in sync with
> hvm_copy_from_guest_phys()'s respective parameter, perhaps both
> would better be paddr_t?
>
Ok. Again I'll fix the prior patches to match.
Paul
> Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-11-24 18:41 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 9:48 [PATCH v2 00/12] viridian: add support for ExProcessorMasks Paul Durrant
2020-11-20 9:48 ` [PATCH v2 01/12] viridian: don't blindly write to 32-bit registers is 'mode' is invalid Paul Durrant
2020-11-20 14:20 ` Jan Beulich
2020-11-20 14:25 ` Durrant, Paul
2020-11-20 9:48 ` [PATCH v2 02/12] viridian: move flush hypercall implementation into separate function Paul Durrant
2020-11-20 9:48 ` [PATCH v2 03/12] viridian: move IPI " Paul Durrant
2020-11-20 9:48 ` [PATCH v2 04/12] viridian: introduce a per-cpu hypercall_vpmask and accessor functions Paul Durrant
2020-11-20 9:48 ` [PATCH v2 05/12] viridian: use hypercall_vpmask in hvcall_ipi() Paul Durrant
2020-11-20 15:09 ` Jan Beulich
2020-11-20 15:12 ` Durrant, Paul
2020-11-20 9:48 ` [PATCH v2 06/12] viridian: use softirq batching " Paul Durrant
2020-11-20 15:11 ` Jan Beulich
2020-11-20 9:48 ` [PATCH v2 07/12] xen/include: import sizeof_field() macro from Linux stddef.h Paul Durrant
2020-11-20 15:15 ` Jan Beulich
2020-11-20 15:24 ` Paul Durrant
2020-11-20 9:48 ` [PATCH v2 08/12] viridian: add ExProcessorMasks variants of the flush hypercalls Paul Durrant
2020-11-24 16:55 ` Jan Beulich
2020-11-24 18:40 ` Paul Durrant
2020-11-20 9:48 ` [PATCH v2 09/12] viridian: add ExProcessorMasks variant of the IPI hypercall Paul Durrant
2020-11-24 16:58 ` Jan Beulich
2020-11-20 9:48 ` [PATCH v2 10/12] viridian: log initial invocation of each type of hypercall Paul Durrant
2020-11-20 9:48 ` [PATCH v2 11/12] viridian: add a new '_HVMPV_ex_processor_masks' bit into HVM_PARAM_VIRIDIAN Paul Durrant
2020-11-20 9:49 ` [PATCH v2 12/12] xl / libxl: add 'ex_processor_mask' into 'libxl_viridian_enlightenment' Paul Durrant
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.