Re: [v5][PATCH 03/10] xen:x86: define a new hypercall to get RMRR mappings

["Chen, Tiejun" <tiejun.chen@intel.com>]

On 2014/9/2 18:15, Jan Beulich wrote:
>>>> On 02.09.14 at 11:59, <tiejun.chen@intel.com> wrote:
>> So could you take a look at the follows?
>
> I'm sorry, but this is a nightmare. Do you know what the term
> "callback" means?

I don't know how to handle this in the hypercall case.

>
>> Then when call the hypercall,
>>
>> +    case XENMEM_reserved_device_memory_map:
>> +    {
>> +        struct xen_mem_reserved_device_memory *xmrdm = NULL;
>> +        struct xen_mem_reserved_device_memory_map xmrdmm;
>> +        XEN_GUEST_HANDLE(xen_mem_reserved_device_memory_t) buffer;
>> +        XEN_GUEST_HANDLE_PARAM(xen_mem_reserved_device_memory_t) buffer_param;
>> +        const struct iommu_ops *ops = iommu_get_ops();
>> +        unsigned int nr_entries = 0;
>> +        unsigned int i = 0;
>> +
>> +        xmrdm = ops->get_device_reserved_memory(&nr_entries);

Do we still need this iommu_ops somewhere else?

>> +        if ( !nr_entries )

Do we still need this 'nr_entries' here?

>> +            return -ENOENT;
>> +        if ( nr_entries < 0 )
>> +            return -EFAULT;
>> +
>> +        if ( copy_from_guest(&xmrdmm, arg, 1) )
>> +            return -EFAULT;
>> +
>> +        if ( xmrdmm.nr_entries < nr_entries )
>> +        {
>> +            xmrdmm.nr_entries = nr_entries;
>> +            if ( copy_to_guest(arg, &xmrdmm, 1) )
>> +                return -EFAULT;
>> +            return -ENOBUFS;
>> +        }
>> +
>> +        buffer_param = guest_handle_cast(xmrdmm.buffer,
>> +                                         xen_mem_reserved_device_memory_t);
>> +        buffer = guest_handle_from_param(buffer_param,
>> +                                         xen_mem_reserved_device_memory_t);
>> +        if ( !guest_handle_okay(buffer, xmrdmm.nr_entries) )
>> +            return -EFAULT;
>> +
>> +        for ( i = 0; i < nr_entries; i++ )
>> +        {
>> +            if ( copy_to_guest_offset(buffer, i, xmrdm + i, 1) )
>> +                return -EFAULT;
>> +        }
>
> The equivalent of this loop would move into the new (abstracted at
> the IOMMU level) VT-d function. Out of that loop you'd call the
> callback passed from here once per iteration; the called function
> would take care of storing the needed information and advancing a
> private counter. No memory allocation, and no VT-d specific
> knowledge required in generic code.
>
> Jan
>
>