Re: [PATCH v3 12/13] xen/arm: Add the property "protected-devices" in the hypervisor node

[Julien Grall <julien.grall@linaro.org>]

Hi Ian,

On 03/18/2014 04:48 PM, Ian Campbell wrote:
> On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote:
>> DOM0 is using the swiotlb to bounce DMA. With the IOMMU support in Xen,
>> protected devices should not use it.
>>
>> Only Xen is abled to know if an IOMMU protects the device. The new property
>> "protected-devices" is a list of device phandles protected by an IOMMU.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> ---
>>     This patch *MUST NOT* be applied until we agreed on a device binding
>>     the device tree folks. DOM0 can run safely with swiotlb on protected
>>     devices while LVM is not used for guest disk.
> 
> LVM works these days I think.

With this patch series applied, LVM will be broken if the hard drive is
protected by an IOMMU. It's the case on midway, the platform will crash
just after the guest begins to boot.

>> @@ -384,6 +387,39 @@ static int make_hypervisor_node(struct domain *d,
>>      if ( res )
>>          return res;
>>  
>> +    if ( kinfo->num_dev_protected )
>> +    {
>> +        /* Don't need to take dtdevs_lock here */
> 
> Why not? Please explain in the comment.

Because, building dom0 is only done with 1 CPU online (e.g CPU0). I
though it was obvious, I will update the comment.

>> +        cells = xmalloc_array(__be32, kinfo->num_dev_protected *
>> +                              dt_size_to_cells(sizeof(dt_phandle)));
>> +        if ( !cells )
>> +            return -FDT_ERR_XEN(ENOMEM);
>> +
>> +        _cells = cells;
> 
> Odd numbers of leading _ are reserved for the compiler IIRC. Even
> numbers are reserved for the libc/environment which is why we can get
> away with such names in hypervisor context.
> 
> But lets just skirt the whole issue and pick a name which doesn't use a
> leading _. cells_iter or c or something.
> 
> Is there no interface to say "make an fdt_property(name, size)"
> returning the data to be filled in?

No, every helpers request to have an input data in parameters.

I will rename _cells into cells_iter.

Regards,

-- 
Julien Grall