Re: [RFC PATCH v2 00/22] xen/arm: Add ITS support

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

On 23/03/15 12:37, Vijay Kilari wrote:
> On Fri, Mar 20, 2015 at 9:53 PM, Julien Grall <julien.grall@linaro.org> wrote:
>> Hi Vijay,
>>
>> On 19/03/2015 14:37, vijay.kilari@gmail.com wrote:
>>>
>>> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>>>
>>> Add ITS support for arm. Following major features
>>> are supported
>>>   - GICv3 ITS support for arm64 platform
>>>   - Supports multi ITS node
>>>   - LPI descriptors are allocated on-demand
>>>   - Only ITS Dom0 is supported
>>>
>>> Tested with single ITS node.
>>
>>
>> Some though about the whole design:
>>
>> Your vGIC ITS driver does too much things. In general a virtual driver
>> should only emulate the hardware for the domain and forward the request to
>> the physical driver.
>>
>> Your series adds device management (create/free) in the vITS, which is
>> wrong.
> 
> The device is added to ITS using MAPD command. All ITS commands are based
> on this device added using MAPD command. So vITS driver needs to manage
> this.

The ITS still have to manage in someway the device. There is lots of
information that doesn't need to be created at every mapd (such as the
number of MSI).

Handling device management in ITS would help to check the validity of
the access. Which you are currently ignoring...

>>
>> How do you check if the domain can use the device?
>> Currently, you allow any domain to use any device. That would bring a big
>> mess with guest using passthrough.
> 
> ITS driver does not know which PCI device is assigned for which domain.

Wrong, Xen knows which device is assigned to which domain so ITS does.

> I think it should be done by above layers along with pci drivers in Xen.
> vITS assume that the domain that sends MAPD command owns the device

The vITS emulates hardware for a specific domain. A malicious guest
could send request to a not own device.

You have to think about security in the vITS otherwise we will end up
with many XSA in this code...

Regards,

-- 
Julien Grall