* [PATCH v3 0/2] x86: VM assist hypercall adjustments
@ 2020-04-21 14:36 Jan Beulich
2020-04-21 14:39 ` [PATCH v3 1/2] x86/HVM: expose VM assist hypercall Jan Beulich
2020-04-21 14:39 ` [PATCH v3 2/2] x86: validate VM assist value in arch_set_info_guest() Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2020-04-21 14:36 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Roger Pau Monné
1: HVM: expose VM assist hypercall
2: validate VM assist value in arch_set_info_guest()
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] x86/HVM: expose VM assist hypercall
2020-04-21 14:36 [PATCH v3 0/2] x86: VM assist hypercall adjustments Jan Beulich
@ 2020-04-21 14:39 ` Jan Beulich
2020-04-22 8:57 ` Julien Grall
2020-04-21 14:39 ` [PATCH v3 2/2] x86: validate VM assist value in arch_set_info_guest() Jan Beulich
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2020-04-21 14:39 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Ian Jackson, George Dunlap, Roger Pau Monné
In preparation for the addition of VMASST_TYPE_runstate_update_flag
commit 72c538cca957 ("arm: add support for vm_assist hypercall") enabled
the hypercall for Arm. I consider it not logical that it then isn't also
exposed to x86 HVM guests (with the same single feature permitted to be
enabled as Arm has); Linux actually tries to use it afaict.
Rather than introducing yet another thin wrapper around vm_assist(),
make that function the main handler, requiring a per-arch
arch_vm_assist_valid_mask() definition instead.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v3: Rename to arch_vm_assist_valid_mask(). Have separate 32- and 64-bit
PV #define-s.
v2: Re-work vm_assist() handling/layering at the same time. Also adjust
arch_set_info_guest().
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -939,6 +939,9 @@ int arch_set_info_guest(
v->arch.dr6 = c(debugreg[6]);
v->arch.dr7 = c(debugreg[7]);
+ if ( v->vcpu_id == 0 )
+ d->vm_assist = c.nat->vm_assist;
+
hvm_set_info_guest(v);
goto out;
}
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -128,6 +128,7 @@ static const hypercall_table_t hvm_hyper
#ifdef CONFIG_GRANT_TABLE
HVM_CALL(grant_table_op),
#endif
+ HYPERCALL(vm_assist),
COMPAT_CALL(vcpu_op),
HVM_CALL(physdev_op),
COMPAT_CALL(xen_version),
--- a/xen/arch/x86/pv/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -57,7 +57,7 @@ const hypercall_table_t pv_hypercall_tab
#ifdef CONFIG_GRANT_TABLE
COMPAT_CALL(grant_table_op),
#endif
- COMPAT_CALL(vm_assist),
+ HYPERCALL(vm_assist),
COMPAT_CALL(update_va_mapping_otherdomain),
COMPAT_CALL(iret),
COMPAT_CALL(vcpu_op),
--- a/xen/common/compat/kernel.c
+++ b/xen/common/compat/kernel.c
@@ -37,11 +37,6 @@ CHECK_TYPE(capabilities_info);
CHECK_TYPE(domain_handle);
-#ifdef COMPAT_VM_ASSIST_VALID
-#undef VM_ASSIST_VALID
-#define VM_ASSIST_VALID COMPAT_VM_ASSIST_VALID
-#endif
-
#define DO(fn) int compat_##fn
#define COMPAT
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1517,20 +1517,23 @@ long do_vcpu_op(int cmd, unsigned int vc
return rc;
}
-#ifdef VM_ASSIST_VALID
-long vm_assist(struct domain *p, unsigned int cmd, unsigned int type,
- unsigned long valid)
+#ifdef arch_vm_assist_valid_mask
+long do_vm_assist(unsigned int cmd, unsigned int type)
{
+ struct domain *currd = current->domain;
+ const unsigned long valid = arch_vm_assist_valid_mask(currd);
+
if ( type >= BITS_PER_LONG || !test_bit(type, &valid) )
return -EINVAL;
switch ( cmd )
{
case VMASST_CMD_enable:
- set_bit(type, &p->vm_assist);
+ set_bit(type, &currd->vm_assist);
return 0;
+
case VMASST_CMD_disable:
- clear_bit(type, &p->vm_assist);
+ clear_bit(type, &currd->vm_assist);
return 0;
}
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -566,13 +566,6 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDL
return -ENOSYS;
}
-#ifdef VM_ASSIST_VALID
-DO(vm_assist)(unsigned int cmd, unsigned int type)
-{
- return vm_assist(current->domain, cmd, type, VM_ASSIST_VALID);
-}
-#endif
-
/*
* Local variables:
* mode: C
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -195,8 +195,6 @@ extern unsigned long frametable_virt_end
#define watchdog_disable() ((void)0)
#define watchdog_enable() ((void)0)
-#define VM_ASSIST_VALID (1UL << VMASST_TYPE_runstate_update_flag)
-
#endif /* __ARM_CONFIG_H__ */
/*
* Local variables:
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -269,6 +269,8 @@ static inline void free_vcpu_guest_conte
static inline void arch_vcpu_block(struct vcpu *v) {}
+#define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
+
#endif /* __ASM_DOMAIN_H__ */
/*
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -309,17 +309,6 @@ extern unsigned long xen_phys_start;
#define ARG_XLAT_START(v) \
(ARG_XLAT_VIRT_START + ((v)->vcpu_id << ARG_XLAT_VA_SHIFT))
-#define NATIVE_VM_ASSIST_VALID ((1UL << VMASST_TYPE_4gb_segments) | \
- (1UL << VMASST_TYPE_4gb_segments_notify) | \
- (1UL << VMASST_TYPE_writable_pagetables) | \
- (1UL << VMASST_TYPE_pae_extended_cr3) | \
- (1UL << VMASST_TYPE_architectural_iopl) | \
- (1UL << VMASST_TYPE_runstate_update_flag)| \
- (1UL << VMASST_TYPE_m2p_strict))
-#define VM_ASSIST_VALID NATIVE_VM_ASSIST_VALID
-#define COMPAT_VM_ASSIST_VALID (NATIVE_VM_ASSIST_VALID & \
- ((1UL << COMPAT_BITS_PER_LONG) - 1))
-
#define ELFSIZE 64
#define ARCH_CRASH_SAVE_VMCOREINFO
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -694,6 +694,25 @@ static inline void pv_inject_sw_interrup
pv_inject_event(&event);
}
+#define PV32_VM_ASSIST_MASK ((1UL << VMASST_TYPE_4gb_segments) | \
+ (1UL << VMASST_TYPE_4gb_segments_notify) | \
+ (1UL << VMASST_TYPE_writable_pagetables) | \
+ (1UL << VMASST_TYPE_pae_extended_cr3) | \
+ (1UL << VMASST_TYPE_architectural_iopl) | \
+ (1UL << VMASST_TYPE_runstate_update_flag))
+/*
+ * Various of what PV32_VM_ASSIST_MASK has isn't really applicable to 64-bit,
+ * but we can't make such requests fail all of the sudden.
+ */
+#define PV64_VM_ASSIST_MASK (PV32_VM_ASSIST_MASK | \
+ (1UL << VMASST_TYPE_m2p_strict))
+#define HVM_VM_ASSIST_MASK (1UL << VMASST_TYPE_runstate_update_flag)
+
+#define arch_vm_assist_valid_mask(d) \
+ (is_hvm_domain(d) ? HVM_VM_ASSIST_MASK \
+ : is_pv_32bit_domain(d) ? PV32_VM_ASSIST_MASK \
+ : PV64_VM_ASSIST_MASK)
+
#endif /* __ASM_DOMAIN_H__ */
/*
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -192,8 +192,6 @@ extern int compat_xsm_op(
extern int compat_kexec_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) uarg);
-extern int compat_vm_assist(unsigned int cmd, unsigned int type);
-
DEFINE_XEN_GUEST_HANDLE(multicall_entry_compat_t);
extern int compat_multicall(
XEN_GUEST_HANDLE_PARAM(multicall_entry_compat_t) call_list,
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -122,8 +122,6 @@ extern void guest_printk(const struct do
__attribute__ ((format (printf, 2, 3)));
extern void noreturn panic(const char *format, ...)
__attribute__ ((format (printf, 1, 2)));
-extern long vm_assist(struct domain *, unsigned int cmd, unsigned int type,
- unsigned long valid);
extern int __printk_ratelimit(int ratelimit_ms, int ratelimit_burst);
extern int printk_ratelimit(void);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] x86: validate VM assist value in arch_set_info_guest()
2020-04-21 14:36 [PATCH v3 0/2] x86: VM assist hypercall adjustments Jan Beulich
2020-04-21 14:39 ` [PATCH v3 1/2] x86/HVM: expose VM assist hypercall Jan Beulich
@ 2020-04-21 14:39 ` Jan Beulich
1 sibling, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2020-04-21 14:39 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Roger Pau Monné
While I can't spot anything that would go wrong, just like the
respective hypercall only permits applicable bits to be set, we should
also do so when loading guest context.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
I'd like to note that Arm lacks a field to save/restore vm_assist.
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -932,6 +932,9 @@ int arch_set_info_guest(
}
}
+ if ( v->vcpu_id == 0 && (c(vm_assist) & ~arch_vm_assist_valid_mask(d)) )
+ return -EINVAL;
+
if ( is_hvm_domain(d) )
{
for ( i = 0; i < ARRAY_SIZE(v->arch.dr); ++i )
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] x86/HVM: expose VM assist hypercall
2020-04-21 14:39 ` [PATCH v3 1/2] x86/HVM: expose VM assist hypercall Jan Beulich
@ 2020-04-22 8:57 ` Julien Grall
2020-04-22 9:04 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2020-04-22 8:57 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
George Dunlap, Roger Pau Monné
Hi Jan,
On 21/04/2020 15:39, Jan Beulich wrote:
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -269,6 +269,8 @@ static inline void free_vcpu_guest_conte
>
> static inline void arch_vcpu_block(struct vcpu *v) {}
>
> +#define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
NIT: Do we want to evaluate d?
Reviewed-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] x86/HVM: expose VM assist hypercall
2020-04-22 8:57 ` Julien Grall
@ 2020-04-22 9:04 ` Jan Beulich
2020-04-22 9:04 ` Julien Grall
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2020-04-22 9:04 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
George Dunlap, xen-devel, Roger Pau Monné
On 22.04.2020 10:57, Julien Grall wrote:
> On 21/04/2020 15:39, Jan Beulich wrote:
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -269,6 +269,8 @@ static inline void free_vcpu_guest_conte
>> static inline void arch_vcpu_block(struct vcpu *v) {}
>> +#define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
>
> NIT: Do we want to evaluate d?
I didn't think we need to, given the very limited use of the
macro.
> Reviewed-by: Julien Grall <jgrall@amazon.com>
Thanks.
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] x86/HVM: expose VM assist hypercall
2020-04-22 9:04 ` Jan Beulich
@ 2020-04-22 9:04 ` Julien Grall
0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2020-04-22 9:04 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
George Dunlap, xen-devel, Roger Pau Monné
On 22/04/2020 10:04, Jan Beulich wrote:
> On 22.04.2020 10:57, Julien Grall wrote:
>> On 21/04/2020 15:39, Jan Beulich wrote:
>>> --- a/xen/include/asm-arm/domain.h
>>> +++ b/xen/include/asm-arm/domain.h
>>> @@ -269,6 +269,8 @@ static inline void free_vcpu_guest_conte
>>> static inline void arch_vcpu_block(struct vcpu *v) {}
>>> +#define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
>>
>> NIT: Do we want to evaluate d?
>
> I didn't think we need to, given the very limited use of the
> macro.
Fair point. I thought I would ask just in case.
>
>> Reviewed-by: Julien Grall <jgrall@amazon.com>
>
> Thanks.
>
> Jan
>
--
Julien Grall
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-22 9:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-21 14:36 [PATCH v3 0/2] x86: VM assist hypercall adjustments Jan Beulich
2020-04-21 14:39 ` [PATCH v3 1/2] x86/HVM: expose VM assist hypercall Jan Beulich
2020-04-22 8:57 ` Julien Grall
2020-04-22 9:04 ` Jan Beulich
2020-04-22 9:04 ` Julien Grall
2020-04-21 14:39 ` [PATCH v3 2/2] x86: validate VM assist value in arch_set_info_guest() Jan Beulich
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.