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

[Julien Grall <julien.grall@xxxxxxxxxx>]

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?
[..]

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