All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@citrix.com>
To: Vijay Kilari <vijay.kilari@gmail.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Prasun Kapoor <Prasun.Kapoor@caviumnetworks.com>,
	Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>,
	Tim Deegan <tim@xen.org>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Stefano Stabellini <stefano.stabellini@citrix.com>,
	manish.jaggi@caviumnetworks.com
Subject: Re: [PATCH v6 17/31] xen/arm: ITS: Store LPIs allocated and IRQ ID bits per domain
Date: Mon, 7 Sep 2015 11:56:18 +0100	[thread overview]
Message-ID: <55ED6D52.1090901@citrix.com> (raw)
In-Reply-To: <CALicx6sm-tsycKfR9spOQFZOpJD_r-YGZnHTR4_AtAqGMdwZsQ@mail.gmail.com>

Hi Vijay,

On 07/09/15 07:59, Vijay Kilari wrote:
> On Thu, Sep 3, 2015 at 9:55 PM, Julien Grall <julien.grall@citrix.com> wrote:
>> I still don't think that it makes sense to introduce the number of LPIs
>> variable in a patch for vITS. As you describe it, it should be part of
>> an ITS/GICv3 patch.
>>
>> Although, I think you should use the field nr_irq_ids in the gic
>> structure (see patch #10) to avoid problem you currently have with this
>> solution.
>>
>> For instance, gic_is_lpi is relying on the number of ID bits exposed by
>> the hardware but you allocate the IRQ desc for LPIs based on nr_lpis.
>>
> 
> You mean to restrict nr_irq_ids to FIRST_GIC_LPI + 8192 (nr_lpis)
> instead of HW supported value?
> and let gic_is_lpi() always check for Xen supported lpi number instead of hw
> supported value?

Yes for both.

>> It's fine to restrict the value in the GIC compare to hardware.
>>
>> Furthermore this value should be 0 on platform where LPIs is not
>> supported in order to make confusion and introduce possible problem with
>> the code later. I could add that, AFAICT, this new variable (nr_lpis) is
>> not that often within the code.
> 
>  nr_lpis is global variable that we define to tell number of LPIs supported
> by Xen. If platform does not support LPIs, then vgic.nr_lpis is set to 0.

While you set vgic.nr_lpis to 0, you don't clear nr_lpis. So
technically, someone could decide to use it later but it won't report
the correct value....

It was difficult to find the correct place to comment about this given
that you've introduced the variable in an odd place....

> 
>>
>> Lastly, on a previous version (don't remember which one) we said that
>> the user should be able to change the value on the command line. What's
>> your plan for that?
> 
> Yes, it was part of our discussion on chat. I will fix it.

A summarize would have been nice... like a TODO in the code and/or in
the cover letter. Depends when you plan to handle it.

Regards,

-- 
Julien Grall

  reply	other threads:[~2015-09-07 10:56 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-31 11:06 [PATCH v6 00/31] Add ITS support vijay.kilari
2015-08-31 11:06 ` [PATCH v6 01/31] xen/dt: Handle correctly node with interrupt-map in dt_for_each_irq_map vijay.kilari
2015-08-31 14:20   ` Julien Grall
2015-09-02 15:28     ` Ian Campbell
2015-09-02 15:30       ` Wei Liu
2015-09-02 15:45       ` Julien Grall
2015-09-02 15:52         ` Ian Campbell
2015-09-04 14:41         ` Ian Campbell
2015-08-31 11:06 ` [PATCH v6 02/31] xen/arm: Add bitmap_find_next_zero_area helper function vijay.kilari
2015-08-31 11:06 ` [PATCH v6 03/31] xen: Add log2 functionality vijay.kilari
2015-08-31 11:21   ` Jan Beulich
2015-08-31 11:06 ` [PATCH v6 04/31] xen/arm: Set nr_cpu_ids to available number of cpus vijay.kilari
2015-08-31 14:25   ` Julien Grall
2015-09-09 12:48     ` Ian Campbell
2015-08-31 11:06 ` [PATCH v6 05/31] xen/arm: Rename NR_IRQs and vgic_num_irqs helper function vijay.kilari
2015-08-31 14:40   ` Julien Grall
2015-09-09 13:08     ` Ian Campbell
2015-09-09 13:23       ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 06/31] xen/arm: ITS: Port ITS driver to Xen vijay.kilari
2015-08-31 15:41   ` Julien Grall
2015-09-03 17:02   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 07/31] xen/arm: ITS: Add helper functions to manage its_devices vijay.kilari
2015-08-31 11:06 ` [PATCH v6 08/31] xen/arm: ITS: Introduce msi_desc for LPIs vijay.kilari
2015-08-31 16:20   ` Julien Grall
2015-09-09 13:16   ` Ian Campbell
2015-09-09 13:28     ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 09/31] xen/arm: ITS: Add APIs to add and assign device vijay.kilari
2015-09-03 17:34   ` Julien Grall
2015-09-09 13:28     ` Ian Campbell
2015-09-09 13:44       ` Julien Grall
2015-09-09 15:07         ` Ian Campbell
2015-09-09 16:19           ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 10/31] xen/arm: ITS: Introduce gic_is_lpi helper function vijay.kilari
2015-08-31 16:49   ` Julien Grall
2015-09-01  9:02     ` Vijay Kilari
2015-09-01 11:40       ` Julien Grall
2015-09-01 11:56         ` Vijay Kilari
2015-09-01 13:02           ` Julien Grall
2015-09-03  6:32             ` Vijay Kilari
2015-09-03  9:48               ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 11/31] xen/arm: ITS: Enable compilation of physical ITS driver vijay.kilari
2015-08-31 11:06 ` [PATCH v6 12/31] xen/arm: Move vgic locking inside get_irq_priority callback vijay.kilari
2015-08-31 16:34   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 13/31] xen/arm: ITS: implement hw_irq_controller for LPIs vijay.kilari
2015-08-31 17:53   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 14/31] xen/arm: ITS: Initialize physical ITS and export lpi support vijay.kilari
2015-08-31 18:35   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 15/31] xen/arm: ITS: Add virtual ITS driver vijay.kilari
2015-09-02 17:20   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 16/31] xen/arm: ITS: Add virtual ITS commands support vijay.kilari
2015-09-03 15:07   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 17/31] xen/arm: ITS: Store LPIs allocated and IRQ ID bits per domain vijay.kilari
2015-09-03 16:25   ` Julien Grall
2015-09-07  6:59     ` Vijay Kilari
2015-09-07 10:56       ` Julien Grall [this message]
2015-08-31 11:06 ` [PATCH v6 18/31] xen/arm: ITS: Enable virtual ITS driver vijay.kilari
2015-08-31 11:06 ` [PATCH v6 19/31] xen/arm: ITS: Export ITS info to Virtual ITS vijay.kilari
2015-09-03 16:48   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 20/31] xen/arm: ITS: Introduce helper to get number of event IDs vijay.kilari
2015-09-03 17:51   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 21/31] xen/arm: ITS: Add GITS registers emulation vijay.kilari
2015-09-07 13:14   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 22/31] xen/arm: ITS: Add virtual ITS availability check helper vijay.kilari
2015-09-07 13:41   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 23/31] xen/arm: ITS: Add 32-bit access to GICR_TYPER vijay.kilari
2015-08-31 16:06   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 24/31] xen/arm: ITS: Add GICR register emulation vijay.kilari
2015-09-07 14:20   ` Julien Grall
2015-09-07 15:26     ` Vijay Kilari
2015-09-09 13:55       ` Ian Campbell
2015-09-09 16:11         ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 25/31] xen/arm: ITS: Allocate irq descriptors for LPIs vijay.kilari
2015-08-31 11:06 ` [PATCH v6 26/31] xen/arm: ITS: Allocate pending_lpi " vijay.kilari
2015-08-31 11:06 ` [PATCH v6 27/31] xen/arm: ITS: Route LPIs vijay.kilari
2015-08-31 11:06 ` [PATCH v6 28/31] xen/arm: ITS: Add domain specific ITS initialization vijay.kilari
2015-08-31 11:06 ` [PATCH v6 29/31] xen/arm: ITS: Map ITS translation space vijay.kilari
2015-08-31 19:07   ` Julien Grall
2015-08-31 11:06 ` [PATCH v6 30/31] xen/arm: ITS: Generate ITS node for Dom0 vijay.kilari
2015-08-31 11:06 ` [PATCH v6 31/31] xen/arm: ITS: Add pci devices in ThunderX vijay.kilari
2015-09-09 15:22   ` Ian Campbell
2015-09-02 15:38 ` [PATCH v6 00/31] Add ITS support Ian Campbell
2015-09-02 15:52   ` Ian Campbell
2015-09-03 16:45 ` Julien Grall
2015-09-09 15:29 ` Ian Campbell
2015-09-14 11:00   ` Vijay Kilari
2015-09-14 11:09     ` Julien Grall
2015-09-14 13:04       ` Vijay Kilari

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55ED6D52.1090901@citrix.com \
    --to=julien.grall@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Prasun.Kapoor@caviumnetworks.com \
    --cc=Vijaya.Kumar@caviumnetworks.com \
    --cc=manish.jaggi@caviumnetworks.com \
    --cc=stefano.stabellini@citrix.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=tim@xen.org \
    --cc=vijay.kilari@gmail.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.