On 23/03/18 15:00, Robin Murphy wrote:
[...]
>> +    /*
>> +     * Treat unknown subtype as RESERVED, but urge users to update their
>> +     * driver.
>> +     */
>> +    if (mem->subtype != VIRTIO_IOMMU_RESV_MEM_T_RESERVED &&
>> +        mem->subtype != VIRTIO_IOMMU_RESV_MEM_T_MSI)
>> +            pr_warn("unknown resv mem subtype 0x%x\n", mem->subtype);
> 
> Might as well avoid the extra comparisons by incorporating this into the 
> switch statement, i.e.:
> 
>       default:
>               dev_warn(vdev->viommu_dev->dev, ...);
>               /* Fallthrough */
>       case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
>               ...
> 
> (dev_warn is generally preferable to pr_warn when feasible)

Alright, that's nicer

[...]
>>      /*
>>       * Last step creates a default domain and attaches to it. Everything
>>       * must be ready.
>> @@ -735,7 +849,19 @@ static int viommu_add_device(struct device *dev)
>>   
>>   static void viommu_remove_device(struct device *dev)
>>   {
>> -    kfree(dev->iommu_fwspec->iommu_priv);
>> +    struct viommu_endpoint *vdev;
>> +    struct iommu_resv_region *entry, *next;
>> +    struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>> +
>> +    if (!fwspec || fwspec->ops != &viommu_ops)
>> +            return;
> 
> Oh good :) I guess that was just a patch-splitting issue. The group 
> thing still applies, though.

Ok

Thanks,
Jean
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to