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

[Julien Grall <julien.grall.oss@xxxxxxxxx>]

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.
> > > 1.2 MAPD Command translation:
> > > -----------------------------------------------
> > >      Format: MAPD device, ITT IPA, ITT Size
> > >
> > >      MAPD is sent with Validation bit set if device needs to be added
> > > and reset when device is removed
> > >
> > > If Validation bit is set:
> > >      - Allocate memory for its_device struct
> > >      - Validate ITT IPA & ITT size and update its_device struct
> > >      - Find number of vectors(nrvecs) for this device by querying PCI
> > > helper function
> > >      - Allocate nrvecs number of LPI
> > >      - Allocate memory for struct vlpi_map for this device. This
> > > vlpi_map holds mapping
> > >        of Virtual LPI to Physical LPI and ID.
> > >      - Find physical ITS node for which this device is assigned
> > >
> > >      - Call p2m_lookup on ITT IPA addr and get physical ITT address
> > >      - Validate ITT Size
> > >      - Generate/format physical ITS command: MAPD, ITT PA, ITT Size
> > >
> > >      Here the overhead is with memory allocation for its_device and
> > > vlpi_map
> >
> > 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...
> > > If Validation bit is not set:
> > >       - Validate if the device exits by checking vITS device list
> > >       - Clear all vlpis assigned for this device
> > >       - Remove this device from vITS list
> > >       - Free memory
> > >
> > > 1.3 MAPVI/MAPI Command translation:
> > > -----------------------------------------------
> > >      Format: MAPVI device, ID, vID, vCID
> > >
> > > - Validate if the device exits by checking vITS device list
> > > - Validate vCID and get pCID by searching cid_map
> > > - if vID does not have entry in vlpi_entries of this device
> > >     If not, Allot pID from vlpi_map of this device and update
> > > vlpi_entries with new pID
> > > - Allocate irq descriptor and add to RB tree
> > > - call route_irq_to_guest() for this pID
> > > - Generate/format physical ITS command: MAPVI device ID, pID, pCID
> > >
> > > Here the overhead is allot physical ID, allocate memory for
> > > irq descriptor and  routing interrupt
> > >
> > > All other ITS command like MOVI, DISCARD, INV, INVALL, INT, CLEAR,
> > > SYNC just validate and generate physical command
> >
> > Interrupt remapping?
> Interrupt mapping is with MAP command. Here as per spec 4.9.22,
> Device, vID should be unique to generate pID. So in case of
> remapping unless Device, vID is changed, new pID is not generated.
4.9.22 for which version of the spec?

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...

Regards,

--
Julien Grall

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