Re: [Xen-devel] [PATCH v3 2/5] x86/hyperv: provide Hyper-V hypercall functions

[Wei Liu <wl@xen.org>]

On Mon, Jan 06, 2020 at 10:38:23AM +0100, Jan Beulich wrote:
[...]
> > +
> > +static inline uint64_t hv_do_rep_hypercall(uint16_t code, uint16_t rep_count,
> > +                                           uint16_t varhead_size,
> > +                                           paddr_t input, paddr_t output)
> > +{
> > +    uint64_t control = code;
> > +    uint64_t status;
> > +    uint16_t rep_comp;
> > +
> > +    control |= (uint64_t)varhead_size << HV_HYPERCALL_VARHEAD_OFFSET;
> > +    control |= (uint64_t)rep_count << HV_HYPERCALL_REP_COMP_OFFSET;
> > +
> > +    do {
> > +        status = hv_do_hypercall(control, input, output);
> > +        if ( (status & HV_HYPERCALL_RESULT_MASK) != HV_STATUS_SUCCESS )
> > +            break;
> > +
> > +        rep_comp = (status & HV_HYPERCALL_REP_COMP_MASK) >>
> > +            HV_HYPERCALL_REP_COMP_OFFSET;
> 
> MASK_EXTR()? (I then also wonder whether MASK_INSR() would better be
> used with some of the other constructs here.)

Sure, I can see if that can be used.

> 
> What's worse though - looking at the definition of
> HV_HYPERCALL_REP_COMP_MASK I notice that it and a few others use
> GENMASK_ULL(), when it was clearly said during review (perhaps of
> another but related patch) that this macro should not be used
> outside of Arm-specific code until it gets put into better shape:
> https://lists.xenproject.org/archives/html/xen-devel/2019-12/msg00705.html

That's a straight import from Linux. I only made the header build
without further inspection.

That can be fixed, of course.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel