Re: [PATCH v8 20/21] xen/arm: Add a hypercall for device mmio mapping

[Julien Grall <julien.grall@arm.com>]

Hi,

On 30/03/16 17:11, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 30, 2016 at 06:08:13PM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> It needs to map platform or amba device mmio to Dom0 on ARM. But when
>> booting with ACPI, it can't get the mmio region in Xen due to lack of
>> AML interpreter to parse DSDT table. Therefore, let Dom0 call a
>> hypercall to map mmio region when it adds the devices.
>>
>> Here we add a new map space like the XEN_DOMCTL_memory_mapping to map
>> mmio region for Dom0. Also add a helper to combine the
>> xsm_add_to_physmap and XENMAPSPACE_dev_mmio space check together.
>>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Keir Fraser <keir@xen.org>
>> Cc: Tim Deegan <tim@xen.org>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>> v8: add a helper to combine xsm_add_to_physmap and XENMAPSPACE_dev_mmio
>> space check together
>
> I was by accident reviewing the earlier ersion. So let me give comments
> here as well.
> .. snipp.

Jan already applied the patch series to xengit/staging. Shannon, can you 
send a follow-up patch to fix at least the printk?

>> +int map_dev_mmio_region(struct domain *d,
>> +                        unsigned long start_gfn,
>> +                        unsigned long nr,
>> +                        unsigned long mfn)
>> +{
>> +    int res;
>> +
>> +    if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1)) )
>> +        return 0;
>> +
>> +    res = map_mmio_regions(d, start_gfn, nr, mfn);
>> +    if ( res < 0 )
>> +    {
>> +        printk(XENLOG_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
>
> Should this be printk ratelimited?

I think so. Today the domain can only be the hardware domain but it may 
change in the future.

>> +               start_gfn, start_gfn + nr - 1, d->domain_id);
>> +        return res;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   int guest_physmap_add_entry(struct domain *d,
>>                               unsigned long gpfn,
>>                               unsigned long mfn,

Regards,

-- 
Julien Grall

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