[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/5] x86/hvm: Misc non-functional cleanup to the HVM_PARAM infrastructure
On Wed, Sep 05, 2018 at 07:12:03PM +0100, Andrew Cooper wrote: > The parameter marshalling and xsm checks are common to both the set and get > paths. Lift all of the common code out into do_hvm_op() and let > hvmop_{get,set}_param() operate on struct xen_hvm_param directly. > > This is somewhat noisy in the functions as each a. reference has to change to > a-> instead. > > In addition, drop an empty default statement, insert newlines as appropriate > between cases, and there is no need to update the IDENT_PT on the fastpath, > because the common path after the switch will make the update. > > No functional change, but a net shrink of -328 to do_hvm_op(). > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> Just two suggestions below. > @@ -4322,41 +4308,34 @@ 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) + > - (0x100 / 8) + (0x10000 / 8) + 1 ) > - a.value = (uint32_t)a.value | > - ((sizeof(struct tss32) + (0x100 / 8) + > - (0x10000 / 8) + 1) << 32); > - a.value |= VM86_TSS_UPDATED; > + if ( (a->value >> 32) > sizeof(struct tss32) + > + (0x100 / 8) + (0x10000 / 8) + 1 ) > + a->value = (uint32_t)a->value | > + ((sizeof(struct tss32) + (0x100 / 8) + > + (0x10000 / 8) + 1) << 32); > + a->value |= VM86_TSS_UPDATED; > break; > > case HVM_PARAM_MCA_CAP: > - rc = vmce_enable_mca_cap(d, a.value); > + rc = vmce_enable_mca_cap(d, a->value); > break; > } > > if ( rc != 0 ) > goto out; > > - d->arch.hvm.params[a.index] = a.value; > + d->arch.hvm.params[a->index] = a->value; > > HVM_DBG_LOG(DBG_LEVEL_HCALL, "set param %u = %"PRIx64, > - a.index, a.value); > + a->index, a->value); > > out: > - rcu_unlock_domain(d); > return rc; If the out label is just return rc, and unless there's no further patches adding code here I would consider removing it. > -static int hvmop_get_param( > - XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg) > +static int hvmop_get_param(struct domain *d, struct xen_hvm_param *a) > { > - struct xen_hvm_param a; > - struct domain *d; > int rc; > > - if ( copy_from_guest(&a, arg, 1) ) > - return -EFAULT; > - > - d = rcu_lock_domain_by_any_id(a.domid); > - if ( d == NULL ) > - return -ESRCH; > - > - rc = -EINVAL; > - if ( !is_hvm_domain(d) ) > - goto out; > - > - rc = hvm_allow_get_param(d, &a); > + rc = hvm_allow_get_param(d, a); You could move this initialization at declaration time. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |