From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [RFC PATCH v2 13/22] xen/arm: its: Add virtual ITS command support Date: Mon, 4 May 2015 14:54:23 +0100 Message-ID: <55477A0F.50003@citrix.com> References: <1426775889-29442-1-git-send-email-vijay.kilari@gmail.com> <553FB23F.7090200@citrix.com> <5540C6D5.9020400@citrix.com> <5540DE16.4090900@citrix.com> <55411D1F.4090308@citrix.com> <5541FF51.6060907@citrix.com> <55423C44.6070702@citrix.com> <55476E72.2010003@citrix.com> <554777CE.6060705@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <554777CE.6060705@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall , Vijay Kilari Cc: Ian Campbell , Stefano Stabellini , Prasun Kapoor , Vijaya Kumar K , Julien Grall , Tim Deegan , "xen-devel@lists.xen.org" , Stefano Stabellini , manish.jaggi@caviumnetworks.com List-Id: xen-devel@lists.xenproject.org On 04/05/2015 14:44, Julien Grall wrote: > Hi Vijay, > > On 04/05/2015 14:27, Vijay Kilari wrote: >> On Mon, May 4, 2015 at 6:34 PM, Julien Grall >> wrote: >>> >>> >>> On 04/05/2015 13:58, Vijay Kilari wrote: >>>> >>>> On Thu, Apr 30, 2015 at 7:59 PM, Julien Grall >>>> 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? Hmmm... I meant for all the domain, not "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