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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.09.13 at 16:31, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> 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 )

Perhaps, but then - as was recommended already - don#t call
the variable "memflags".

Jan


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