From: Ian Campbell <ian.campbell@citrix.com>
To: Julien Grall <julien.grall@citrix.com>,
vijay.kilari@gmail.com, stefano.stabellini@eu.citrix.com,
stefano.stabellini@citrix.com, tim@xen.org,
xen-devel@lists.xen.org
Cc: Prasun.Kapoor@caviumnetworks.com,
Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>,
manish.jaggi@caviumnetworks.com
Subject: Re: [PATCH v6 09/31] xen/arm: ITS: Add APIs to add and assign device
Date: Wed, 9 Sep 2015 16:07:53 +0100 [thread overview]
Message-ID: <1441811273.24450.334.camel@citrix.com> (raw)
In-Reply-To: <55F037AD.6060606@citrix.com>
On Wed, 2015-09-09 at 14:44 +0100, Julien Grall wrote:
> On 09/09/15 14:28, Ian Campbell wrote:
> > On Thu, 2015-09-03 at 18:34 +0100, Julien Grall wrote:
> > > @@ -522,6 +535,205 @@ static void its_lpi_free(struct its_device
> > > *dev)
> > > > xfree(dev->event_map.lpi_map);
> > > > }
> > > >
> > > > +static void its_discard_lpis(struct its_device *dev, u32 ids)
> > > > +{
> > > > + int i;
> > > > +
> > >
> > > I would have expected a function more complex than that. If you
> > > discard
> > > the LPIs, you also need to free the MSI desc and potentially reset
> > > the
> > > IRQ desc.
> > >
> > > Otherwise you will left the irq_desc in an unknown state for the next
> > > one.
> >
> > But discard != free? (or at least "discard" has a specific meaning in
> > its).
>
> In the ITS, discard means removing the mapping from the MSI (eventID) to
> the LPI.
Table 6-6 in the gic arch spec (ARM IHI 0069A (ID060315)) says about the
discard ITS command "Translates the event defined by EventID and DeviceID
and instructs the appropriate Redistributor to remove the pending state of
the interrupt. It also ensures that any caching in the Redistributors
associated with a specific EventID is consistent with the configuration
held in memory."
And section 6.3.4 seems to agree _except_ for the pseudo code which does
indeed seem to include setting the corresponding ITE entry to invalid.
Given that the pseudocode for CLEAR appears to match how DISCARD is
decribed (infact the descrpitions of CLEAR and DISCARD look functionally
identical to me) I'm inclined to believe the pseudo code for DISCARD, i.e.
the docs are misleading and you are correct that DISCARD is more than just
clearing the pending state.
The older PRD03-GENC-010745 24.0 looks to be more correct.
So sorry for the noise, that'll teach me to believe docs.
> > > > + xfree(dev->event_map.col_map);
> > > > + xfree(dev);
> > > > +}
> > > > +
> > > > +static struct its_device *its_alloc_device(u32 devid, u32 nr_ites,
> > > > + struct dt_device_node
> > > > *dt_its)
> > > > +{
> > > > + struct its_device *dev;
> > > > + paddr_t *itt;
> > >
> > > Why paddr_t? You only allocate it and pass directly to the hardware.
> >
> > paddr_t seems correct, it the fact that it is a paddr_t * (i.e. a
> > pointer)
> > which seems odd to me. I think you commented the same thing on dev
> > ->itt_addr which is where this ends up assigned.
>
> With the current usage, we store the result of xmalloc in the itt
> variable. So it's a pointer to a virtual address not a paddr_t.
Ah, in that case it should indeed have some other type.
> If we decide to use paddr_t, then we should also update the code.
> Although, the Linux ITS code is using void * in order to store the
> pointer. So I'd like to keep the same in order to avoid differing too
> much for Linux (though with all this coding style it would be difficult
> to backport code).
>
> Regards,
>
next prev parent reply other threads:[~2015-09-09 15:07 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 [this message]
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
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=1441811273.24450.334.camel@citrix.com \
--to=ian.campbell@citrix.com \
--cc=Prasun.Kapoor@caviumnetworks.com \
--cc=Vijaya.Kumar@caviumnetworks.com \
--cc=julien.grall@citrix.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.