[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Xen on ARM vITS Handling Draft B (Was Re: Xen/arm: Virtual ITS command queue handling)



On Mon, May 25, 2015 at 6:14 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hi,
>
>
> On 25/05/2015 12:40, Vijay Kilari wrote:
>>
>> On Mon, May 25, 2015 at 3:02 PM, Julien Grall
>> <julien.grall.oss@xxxxxxxxx> wrote:
>>>
>>>
>>>
>>> On 25/05/2015 11:06, Vijay Kilari wrote:
>>>>
>>>>
>>>> On Sun, May 24, 2015 at 4:05 PM, Julien Grall <julien.grall@xxxxxxxxxx>
>>>> wrote:
>>>>>>
>>>>>>
>>>>>> 1) Command translation:
>>>>>> -----------------------------------
>>>>>>
>>>>>>     - ITS commands contains device ID, Event ID (vID), Collection ID
>>>>>> (vCID), Target Address (vTA)
>>>>>>        parameters
>>>>>>     - All these parameters should be validated
>>>>>>     - These parameters should be translated from Virtual to Physical
>>>>>>
>>>>>> Of the existing GICv3 ITS commands, MAPC, MAPD, MAPVI/MAPI are the
>>>>>> time
>>>>>> consuming commands as these commands creates entry in the Xen ITS
>>>>>> structures,
>>>>>> which are used to validate other ITS commands.
>>>>>>
>>>>>> 1.1 MAPC command translation
>>>>>> -----------------------------------------------
>>>>>>       Format: MAPC vCID, vTA
>>>>>>
>>>>>>       -  vTA is validated against Re-distributor address by searching
>>>>>> Redistributor region /
>>>>>>           CPU number based on GITS_TYPER.PAtype and Physical
>>>>>> Collection
>>>>>> ID & Physical
>>>>>>           Target address are retrieved
>>>>>>       -  Each vITS will have cid_map (struct cid_mapping) which holds
>>>>>> mapping of
>>>>>>          Virtual Collection ID, Virtual Targets address and Physical
>>>>>> Collection ID.
>>>>>>       -  MAPC pCID, pTA physical ITS command is generated
>>>>>>
>>>>>>       Here there is no overhead, the cid_map entries (approx 32
>>>>>> entries)
>>>>>> are preallocated when
>>>>>>       vITS is created.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> How did you decide the 32 entries? The ITS must at least provide N + 1
>>>>> collection when N is the number of processors.
>>>>
>>>>
>>>>
>>>> It should be MAX_VIRT_VCPUS.
>>>
>>>
>>>
>>> Why not allocating dynamically rather than wasting memory?
>>>
>>>>>
>>>>> Also, how do you handle collection re-mapping?
>>>>
>>>>
>>>>
>>>> There is one collection per cpu. The vTA of MAPC should fall within
>>>> vcpus range (GITS_TYPE.PTAtype is 0).
>>>
>>>
>>>
>>> It's not what I asked...
>>>
>>>> In case of remapping,  if the vCID does not exists in cid_map,
>>>> then new entry is made (vCID, pCID, vTA)
>>>>
>>>> If vCID exists, the existing entry is updated with pCID, vTA
>>>>
>>>> However this cid_map should be used to inject to right pCPU where
>>>> vCPU is running.
>>>
>>>
>>>
>>> What do you mean by injecting? The MAPC should never be injected to the
>>> physical CPU. As I said earlier, the collection is shared with all the
>>> vCPU
>>> and Xen.
>>>
>>
>> It does not mean MAPC is sent to physical CPU,
>>
>> All interrupts mapped to collection are taken on cpus 0 to nr_vcpus.
>> when vCID is mapped to pCID, all pCID fall in the range of 0 to nr_vcpus
>
>
> vCID can be higher than the number of VCPUs (the vITS has to support
> nr_vcpus + 1 collection).
>
> Also, the number of physical collection may be lower than the virtual
> collection because the user created a guest with num vCPUs > num pCPU.
>
>> So, irrespective of vcpus running on physical cpus all interrupts are
>> routed
>> to pCPU 0 to nr_vcpus
>>
>> Similar to below patch done for SPIs. LPIs should also be injected.
>
>
> I know that LPIs should be injected...
>
>>
>> http://lists.xen.org/archives/html/xen-devel/2014-09/msg04176.html
>>
>> Correct me if I have not understood your question correctly.
>
>
> AFAIU your proposal, the function mapping(vCID) will always return the same
> pCID, right?

Yes, vCID to pCID is mapped

>
> [..]
>
>>>>> What about device remapping?
>>>>
>>>>
>>>>
>>>> IMO, device cannot be remapped. It has to removed (MAPD with valid bit
>>>> 0)
>>>> so that ITS HW can remove the entries and added with new MAPD command.
>>>
>>>
>>>
>>> Your opinion is not the spec...
>>>
>>> Device remapping is allowed by the spec (see 4.9.18 "Re-mapping and
>>> Un-mapping devices in PRD03-GENC-010745 24.0). So even it's not possible
>>> (with a spec ref in proof), you have to protect it...
>>
>>
>> I am no saying that is my opinion, I mean the same as told in 4.9.18,
>
>
> IMO === In My Opinion... I can't guess that you were talking about 4.9.18.
>
>> To unmap the device, the MAPD should be sent with valid bit 0, which will
>
>
> s/unmap/re-map/ ?
>
>> remove the device from the list and added again on MAPD with valid bit 1
>
>
> I can't see where the spec says that 2 MAPD (one with V=1 and the other with
> V=0) is required. The section 4.9.18 contains an 'or':
>
> "Issue a mapping command (MAPD; see section 5.13.11) or an un-mapping
> command"
>
> This is related to "Interrupts can be re-mapped or un-mapped".
>
> 4.9.18 and 5.13.11 (PRD03-GENC-010745 24.0) are only speaking about a single
> MAPD:
>
> "Note: software might issue a MAPD command to re-map an already mapped
> device and the ITS must invalidate all cached data for that device."
>

OK. I have missed this 'or'. If so, MAPD always overwrites the old info

>>>
>>> new pID may not be re-generated but there is some care to take when an
>>> vID
>>> is remapped. (see 4.9.17 "Re-mapping and Un-mapping Interrupts" in
>>> PRD03-GENC-010745 24.0).
>>>
>>>> If vCID is changed, a new pCID is generated based on MAPC command
>>>
>>>
>>>
>>> Which is wrong...
>>
>>
>> When you say vID is remapped, then vCID should be different right?
>
>
> Yes. I was confuse by "MAPC command" at the end.
>
> Regards,
>
> --
> Julien Grall

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.