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

Re: [Xen-devel] [Draft C] Xen on ARM vITS Handling



Hi Ian,

On 01/06/15 14:36, Ian Campbell wrote:
On Fri, 2015-05-29 at 15:06 +0100, Julien Grall wrote:
Hi Vijay,

On 27/05/15 17:44, Vijay Kilari wrote:
## Command Translation

Of the existing GICv3 ITS commands, `MAPC`, `MAPD`, `MAPVI`/`MAPI` are
potentially time consuming commands as these commands creates entry in
the Xen ITS structures, which are used to validate other ITS commands.

`INVALL` and `SYNC` are global and potentially disruptive to other
guests and so need consideration.

All other ITS command like `MOVI`, `DISCARD`, `INV`, `INT`, `CLEAR`
just validate and generate physical command.

### `MAPC` command translation

Format: `MAPC vCID, vTA`

   -  The GITS_TYPER.PAtype is emulated as 0. Hence vTA is always represents
      vcpu number. Hence vTA is validated against physical Collection
IDs by querying
      ITS driver and corresponding Physical Collection ID is retrieved.
   -  Each vITS will have cid_map (struct cid_mapping) which holds mapping of

Why do you speak about each vITS? The emulation is only related to one
vITS and not shared...

And each vITS will have a cid_map, which is used. This seems like a
reasonable way to express this concept in the context.

This is rather strange when everything in the command emulation is per-vits.

Perhaps there is a need to include discussion of some of the secondary
data structures alongside the defintion `cits_cq`. In which case we
could talk about "its associated `cid_map`" and things.

      Virtual Collection ID(vCID), Virtual Target address(vTA) and
      Physical Collection ID (pCID).
      If vCID entry already exists in cid_map, then that particular
mapping is updated with
      the new pCID and vTA else new entry is made in cid_map

When you move a collection, you also have to make sure that all the
interrupts associated to it will be delivered to the new target.

I'm not sure what you are suggesting for that...

This is going to be rather painful I fear.

   -  MAPC pCID, pTA physical ITS command is generated

We should not send any MAPC command to the physical ITS. The collection
is already mapped during Xen boot and the guest should not be able to
move the physical collection (they are shared between all the guests and
Xen).

This needs discussion in the background section, to describe the
physical setup which the virtual stuff can make assumption of.

I don't think this is a background section. The physical number of
collection is limited (the mandatory number of collections is nr_cpus +
1). Those collection will likely be shared between Xen and the different
guests.

If we let the guest moving the physical collection we will also move all
the interrupts which is wrong.


   Here there is no overhead, the cid_map entries are preallocated
with size of nr_cpus
   in the platform.

As said the number of collection should be at least nr_cpus + 1.

FWIW I read this as "with size appropriate for nr_cpus", which leaves
the +1 as implicit. I added the +1 nevertheless.

I wanted to make clear. His implementation was only considering nr_cpus
collections.


- `MAPC pCID, pTA` physical ITS command is generated

### `MAPD` Command translation

Format: `MAPD device, Valid, ITT IPA, ITT Size`

`MAPD` is sent with `Valid` bit set if device needs to be added and reset
when device is removed.

If `Valid` 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 XXX nrvecs is a function of `ITT Size`?
- 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 with which this device is associated
- 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`

XXX Suggestion was to preallocate some of those at device passthrough
setup time?

If Validation bit is set:
   - Query its_device tree and get its_device structure for this device.
   - (XXX: If pci device is hidden from dom0, does this device is added
       with PHYSDEVOP_pci_device_add hypercall?)
   - If device does not exists return
   - If device exists in RB-tree then
          - Validate ITT IPA & ITT size and update its_device struct

To validate the ITT size you need to know the number of interrupt ID.

Please could you get into the habit of making concrete suggestions for
changes to the text. I've no idea what change I should make based on
this observation. If not concrete suggestions please try and make the
implications of what you are saying clear.

The size of the ITT is based on the number of Interrupt supported by the
device.

The only way to validate the size getting the number of Interrupt
before. i.e

        - Find the number of MSI for this device
        - Compute the theorical size of the ITT
        - Validate the ITT IPA and the ITT size
        - Update the its_device struct

Note: this is based on this conversation and doesn't take into account
the other threads.




          - Check if device is already assigned to the domain,
            if not then
               - Find number of vectors(nrvecs) for this device.
               - Allocate nrvecs number of LPI
               - Fetch vlpi_map for this device (preallocated at the
time of adding
                 this device to Xen). This vlpi_map holds mapping of
Virtual LPI to
                 Physical LPI and ID.
               - Call p2m_lookup on ITT IPA addr and get physical ITT address
               - Assign this device to this domain and mark as enabled
          - If this device already exists with the domain (Domain is
remapping the device)
               - Validate ITT IPA & ITT size and update its_device struct
               - Call p2m_lookup on ITT IPA addr and get physical ITT address
               - Disable all the LPIs of this device by searching
through vlpi_map and LPI
                 configuration table

Disabling all the LPIs associated to a device can be time consuming
because you have to unroute them and make sure that the physical ITS
effectively disabled it before sending the MAPD command.

Given that the software would be buggy if it send a MAPD command without
releasing all the associated interrupt we could ignore the command if
any interrupt is still enabled.

Releasing how? Did you mean disable?

I think disable is enough (see 4.9.18).


    - Clear all vlpis mapping for this device
    - Remove this device from the domain

### `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 allocate
  a new 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`


- Validate if the device exists by checking vITS device RB-tree.
- Validate vCID and get pCID by searching cid_map
- if vID does not have entry in vlpi_entries of this device
      -  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
  If exists,
     - If vCID is different ( remapping interrupts to differnt collection ),
            - Disable LPI

You have to ensure the the LPI is disabled with an INV/SYNC.

Can we not rely on the subsequent INV/SYNC from the guest relating to
the MAPI here?

I think you meant MAPVI here.

It's not mandatory that MAPVI is followed by an INV. INV is only used when the software modified the LPI configuration table.

Furthermore, it's not clear what would be the behavior of the ITS if the interrupt is not correctly disabled (i.e not having an INV/SYNC before MAPI).

Although, I think more about it and Xen should not disable/enable any IRQ without the explicit request of the guest. Based on 4.9.17, I think we can ignore the command if the IRQ has not been disabled (i.e LPI disabled in the LPI conf table + INV/INVALL sent).


Although as suggested on the previous commands, we may want to deny the
command if the interrupt is not disabled.

            - Update the vlpi_map
             (XXX: Enable LPI on guest request?)

Read the spec...

If you know the answer then please share it rather than making people
guess what you are talking about.

The Spec is pretty clear on it (see 4.9.17). The guest is managing the
interrupt itself via the LPI configuration table, we should not try to
do more than the guest ask.

The question to enable/disable the IRQ will depend on the virtual LPI
configuration table and if we already replicated the value to the
physical one.

Although as we don't have any mapping vLPI -> pLPI at this time, it likely means that we have to check the virtual LPI configuration table, replicate the value in the physical table and then send an INV/SYNC (if the value has been toggled) to ensure the completion.

This is because it would be valid, even if it's pointless, to enable an LPI before mapping a (DevID, ID).

Regards,

--
Julien Grall

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