On 18/12/2019 19:40, Tamas K Lengyel wrote:
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 614ed60fe4..5a3a962fbb 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4072,16 +4072,17 @@ static int hvmop_set_evtchn_upcall_vector(
> }
>
> static int hvm_allow_set_param(struct domain *d,
> - const struct xen_hvm_param *a)
> + uint32_t index,
> + uint64_t new_value)
These two lines can be folded together.
> @@ -4134,13 +4135,11 @@ static int hvm_allow_set_param(struct domain *d,
> return rc;
> }
>
> -static int hvmop_set_param(
> +int hvmop_set_param(
> XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg)
and here.
> @@ -4160,23 +4159,42 @@ static int hvmop_set_param(
> if ( !is_hvm_domain(d) )
> goto out;
>
> - rc = hvm_allow_set_param(d, &a);
> + rc = hvm_set_param(d, a.index, a.value);
> +
> + out:
> + rcu_unlock_domain(d);
> + return rc;
> +}
> +
> +int hvm_set_param(
> + struct domain *d,
> + uint32_t index,
> + uint64_t value)
and again.
> @@ -4340,34 +4358,33 @@ static int hvmop_set_param(
> * 256 bits interrupt redirection bitmap + 64k bits I/O bitmap
> * plus one padding byte).
> */
> - if ( (a.value >> 32) > sizeof(struct tss32) +
> + if ( (value >> 32) > sizeof(struct tss32) +
> (0x100 / 8) + (0x10000 / 8) + 1 )
> - a.value = (uint32_t)a.value |
> + value = (uint32_t)value |
> ((sizeof(struct tss32) + (0x100 / 8) +
> (0x10000 / 8) + 1) << 32);
Can you reindent the surrounding lines so they line up again?
> @@ -4429,42 +4446,60 @@ static int hvmop_get_param(
> if ( !is_hvm_domain(d) )
> goto out;
>
> - rc = hvm_allow_get_param(d, &a);
> + rc = hvm_get_param(d, a.index, &a.value);
> if ( rc )
> goto out;
>
> - switch ( a.index )
> + rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> +
> + HVM_DBG_LOG(DBG_LEVEL_HCALL, "get param %u = %"PRIx64,
> + a.index, a.value);
> +
> + out:
> + rcu_unlock_domain(d);
> + return rc;
> +}
> +
> +int hvm_get_param(
> + struct domain *d,
> + uint32_t index,
> + uint64_t *value)
Fold.
> +{
> + int rc;
> +
> + if ( index >= HVM_NR_PARAMS || !value )
No need for !value. It had better only ever point onto the hypervisor
stack now that this is an internal function.
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 1d7b66f927..a6f4ae76a1 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -335,6 +335,10 @@ unsigned long hvm_cr4_guest_valid_bits(const struct
> domain *d, bool restore);
> bool hvm_flush_vcpu_tlb(bool (*flush_vcpu)(void *ctxt, struct vcpu *v),
> void *ctxt);
>
> +/* Caller must hold domain locks */
How about asserting so?
No major problems so Acked-by: Andrew Cooper <andrew.coop...@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
