Re: [Xen-devel] [PATCH v6 5/5] xen: introduce XENMEM_pin

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 30 Sep 2013, Jan Beulich wrote:
> >>> On 30.09.13 at 14:56, Stefano Stabellini 
> >>> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > On Mon, 30 Sep 2013, Jan Beulich wrote:
> >> > +
> >> > +    if ( !is_hardware_domain(d) )
> >> > +    {
> >> > +        rc = -EPERM;
> >> > +        goto fail;
> >> > +    }
> >> > +
> >> > +    memflags |= MEMF_bits(domain_clamp_alloc_bitsize(
> >> > +        d,
> >> > +        XENMEMF_get_address_bits(pin.in.mem_flags) ? :
> >> > +        (BITS_PER_LONG+PAGE_SHIFT)));
> >> > +
> >> > +    for ( ; i < pin.in.nr_extents; i++ )
> >> > +    {
> >> > +        if ( unlikely(__copy_from_guest_offset(
> >> > +                      &gpfn, pin.in.extent_start, i, 1)) )
> >> > +        {
> >> > +            rc = -EFAULT;
> >> > +            goto fail;
> >> > +        }
> >> > +
> >> > +        if ( generic_fls64(gpfn << PAGE_SHIFT) > memflags )
> >> 
> >> Didn't you mean MEMF_bits(memflags) here?
> > 
> > memflags is set to MEMF_bits(XXX) in the assignment above
> 
> No, the bit width gets or-ed into memflags - that's no the same.

The usage of MEMF_bits is indeed wrong: MEMF_bits(n) is
((n)<<_MEMF_bits). Given that XENMEMF_get_address_bits(pin.in.mem_flags)
is the most significant bit allowed in the page address, I think we only
need:

    memflags = domain_clamp_alloc_bitsize(d,
    XENMEMF_get_address_bits(pin.in.mem_flags) ? :
    (BITS_PER_LONG+PAGE_SHIFT));
    
    [...]
    
    if ( generic_fls64(gpfn << PAGE_SHIFT) > memflags )

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel