On Mon, Apr 23, 2018 at 09:11:28AM +0200, Cédric Le Goater wrote: > On 04/23/2018 05:59 AM, David Gibson wrote: > > On Fri, Apr 20, 2018 at 10:27:21AM +0200, Cédric Le Goater wrote: > >> On 04/20/2018 09:10 AM, David Gibson wrote: > >>> On Thu, Apr 19, 2018 at 02:42:57PM +0200, Cédric Le Goater wrote: > >>>> Each XIVE interrupt source is associated with a two bit state machine > >>>> called an Event State Buffer (ESB) : the first bit "P" means that an > >>>> interrupt is "pending" and waiting for an EOI and the bit "Q" (queued) > >>>> means a new interrupt was triggered while another was still pending. > >>>> > >>>> When an event is triggered, the associated interrupt state bits are > >>>> fetched and modified and forwarded to the virtualization engine of the > >>>> controller doing the routing. These can also be controlled by MMIO, to > >>>> trigger events or turn off the sources for instance. See code for more > >>>> details on the states and transitions. > >>>> > >>>> On a sPAPR machine, the OS will obtain the address of the MMIO page of > >>>> the ESB entry associated with a source and its characteristic using > >>>> the H_INT_GET_SOURCE_INFO hcall. On PowerNV, a similar OPAL call is > >>>> used. > >>>> > >>>> The xive_source_notify() routine is in charge forwarding the source > >>>> event notification to the routing engine. It will be filled later on. > >>>> > >>>> Signed-off-by: Cédric Le Goater <c...@kaod.org> > >>>> --- > >>>> Changes since v2: > >>>> > >>>> - added support for Store EOI > >>>> - added support for two page MMIO setting like on KVM > >>> > >>> Looks generally sane to me, though I have a few queries. [snip] > >>>> + * The default XIVE interrupt source setting for ESB MMIO is two 64k > >>>> + * pages without Store EOI. This is in sync with KVM. > >>>> + */ > >>>> +static Property xive_source_properties[] = { > >>>> + DEFINE_PROP_UINT64("flags", XiveSource, esb_flags, 0), > >>>> + DEFINE_PROP_UINT32("nr-irqs", XiveSource, nr_irqs, 0), > >>>> + DEFINE_PROP_UINT64("bar", XiveSource, esb_base, 0), > >>> > >>> Isn't this redundant with however the base address is handled through > >>> the SysBusDevice stuff (I forget the details)? > >> > >> Storing the ESB MMIO base address under the XiveSource object is > >> convenient later on in the h_int_get_source_info hcall which make > >> use of the helpers : > >> > >> hwaddr xive_source_esb_trigger(XiveSource *xsrc, uint32_t srcno) > >> hwaddr xive_source_esb_mgmt(XiveSource *xsrc, int srcno) > >> > >> But it is only used in that place. So we could just store the ESB > >> MMIO base address under the sPAPRXive controller. This makes some > >> sense in the design, as we have to inform KVM of this address with > >> a KVM device ioctl. > > > > Well.. I really dislike the idea that you could change the actual MMIO > > mapping address in one place, but other bits of code would still think > > it was mapped somewhere else. > > OK. I think that my last proposal of removing the ESB MMIO address > from the source and letting the owning device, the sPAPR Xive controller > in this case, but this is the same for PoweNV or PSI HB, handle the > mapping goes in the direction you want to take ? > > It does looks better in the overall XIVE model and the XiveSource > object have no need of this address.
Yes, I think that's the way to go. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature