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

Re: [Xen-devel] [RFC PATCH v2 13/22] xen/arm: its: Add virtual ITS command support



Hi Vijay,

On 04/05/2015 14:27, Vijay Kilari wrote:
On Mon, May 4, 2015 at 6:34 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:


On 04/05/2015 13:58, Vijay Kilari wrote:

On Thu, Apr 30, 2015 at 7:59 PM, Julien Grall <julien.grall@xxxxxxxxxx>
wrote:

Hi,

On 30/04/15 14:47, Stefano Stabellini wrote:


If the devid is not within this range, the ITS won't recognize the
value and
won't be able to send the interrupt.

So this is clearly not the right value.


Sure, in that case the maximum value allowed by GITS_TYPER.Devbits.
Vijay, what is the value of GITS_TYPER.Devbits on your platform?


It is 21 bits


I would imagine that 21 bits would be plenty to find an unused devid.

Alternatively we could use an inexistent function of a real device, such
as 00:00.1 (function 1 of the host bridge).


As discussed IRL, this idea sounds good to me.

Although I would be happy with any other way which ensure the devid is
free.


Has prototyped with 00.00.1 as device id. But I see that Dom0 boot is
slow compared to polling mode. This could be because Dom0 has to keep
trapping on creader to check if creader is updated or not.


How did you implement the interrupt mode? Could it be improve?

    1) In physical ITS driver its_device is created with devID 00:00.1 with
256 MSI-x are reserved and is named as completion_dev, which is global.

That's a lot of MSI-x reserved... Can't you use only one per domain?

    2) In Domain init,
          - one irq (called completion_irq) is allocated per domain for
this device

So only 256 domain can run on your platform at the same time?

            and irq desc is allocated to this domain. This way we can get vITS
            context when interrupt is received.
          - An array of 32 requests(its_requests) is allocated which stores all
            the information about the ITS commands that are converted and 
written
            to physical ITS queue and each request info contains
            [CReader vITS] [CWriter vITS] [Physical Q Index] [Number of 
commands]
             [Completion irq] [ status ]

Why 32 requests?

Also some of the fields don't make much sense to me such as "Number of Commands" , "Completion IRQ" and "status" I guess I will find out when you will send the new series.

    3) When vITS received ITS command. This command is converted to physical
       command  and written to physical queue, INT command is appended with
       completion_irq and entry is made in its_requests.
    4) On receiving completion_irq, vITS structure and its_requests
info is parsed
        and Creader of vITS is upstated with [ Cwriter vITS ] stored
for this request
        and this request is removed from its_requests.

You complicate the code by allowing 32 batch of command per domain (looping is very slow). You should only allow one batch of command per domain. When the batch is finished you can send another one.

  I am adding one INT per command. This can be improved to add one INT
cmd for all
  the pending commands. Existing Linux driver sends 2 commands at a time.

You should not assume that other OS will send 2 commands at the same time... It could be more or less.

Although, having a INT per command is rather slow. One INT command per batch would improve the boot time.

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