[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 07/26] ppc/xive: add MMIO handlers to the XI
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [RFC PATCH 07/26] ppc/xive: add MMIO handlers to the XIVE interrupt source |
Date: |
Tue, 25 Jul 2017 22:21:54 +1000 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Mon, Jul 24, 2017 at 05:55:42PM +0200, Cédric Le Goater wrote:
> On 07/24/2017 06:29 AM, David Gibson wrote:
> > On Wed, Jul 05, 2017 at 07:13:20PM +0200, Cédric Le Goater wrote:
> >> Each interrupt source is associated with a 2-bit state machine called
> >> an Event State Buffer (ESB). It is controlled by MMIO to trigger
> >> events.
[snip]
> >> +/* TODO: handle second page */
> >
> > Is this comment still relevent?
>
> Some HW have a second page to trigger the event. I am not sure we need
> to model it though. I will make some inquiries.
Ah, ok. Maybe clarify the comment a bit.
[snip]
> >> +static void xive_esb_write(void *opaque, hwaddr addr,
> >> + uint64_t value, unsigned size)
> >> +{
> >> + XiveICSState *xs = ICS_XIVE(opaque);
> >> + XIVE *x = xs->xive;
> >> + uint32_t offset = addr & 0xF00;
> >> + uint32_t srcno = addr >> xs->esb_shift;
> >> + uint32_t lisn = srcno + ICS_BASE(xs)->offset;
> >> + XiveIVE *ive;
> >> + bool notify = false;
> >> +
> >> + ive = xive_get_ive(x, lisn);
> >> + if (!ive || !(ive->w & IVE_VALID)) {
> >> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid LISN %d\n", lisn);
> >> + return;
> >> + }
> >
> > Having this code associated with the individual ICS look directly at
> > the IVE table in the core xive object seems a bit dubious.
>
> The IVE table holds the validity and mask status of the interrupt
> entries, so we need that lookup. However, (continues below) ...
>
> > This also
> > points out another mismatch between the re-used ICS code and the new
> > XIVE code: ICS gathers all the per-source-irq flags/state into the
> > irqstate structure, whereas xive has per-irq information in the
> > centralized ecb and IVE tables. There can certainly be good reasons
> > for that, but using both at once is kind of clunky.
>
> I understand that you would rather put the esbs in the source they
> belong to. That is the case on real HW but it makes the modeling a
> bit more difficult. We would need to choose a MMIO address to give
> to the guest OS. I had some issues with the allocator (I need
> to look at this problem closer).
Uh.. what do MMIO addresses have to do with this? I'm talking about
the actual ESB state in the packed bit array.
> It might also be an "issue" for KVM. Ben talked about maintaining
> all the esbs of a guest under a single memory region to be able to
> map the pages in the host.
>
> Any how, I agree this is another point to discuss in the sPAPR
> model.
>
> Thanks,
>
> C.
>
>
> >> + if (srcno >= ICS_BASE(xs)->nr_irqs) {
> >> + qemu_log_mask(LOG_GUEST_ERROR,
> >> + "XIVE: invalid IRQ number: %d/%d lisn: %d\n",
> >> + srcno, ICS_BASE(xs)->nr_irqs, lisn);
> >> + return;
> >> + }
> >> +
> >> + switch (offset) {
> >> + case 0:
> >> + /* TODO: should we trigger even if the IVE is masked ? */
> >> + notify = xive_pq_trigger(x, lisn);
> >> + break;
> >> + default:
> >> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid ESB write addr
> >> %d\n",
> >> + offset);
> >> + return;
> >> + }
> >> +
> >> + if (notify && !(ive->w & IVE_MASKED)) {
> >> + qemu_irq_pulse(ICS_BASE(xs)->qirqs[srcno]);
> >> + }
> >> +}
> >> +
> >> +static const MemoryRegionOps xive_esb_ops = {
> >> + .read = xive_esb_read,
> >> + .write = xive_esb_write,
> >> + .endianness = DEVICE_BIG_ENDIAN,
> >> + .valid = {
> >> + .min_access_size = 8,
> >> + .max_access_size = 8,
> >> + },
> >> + .impl = {
> >> + .min_access_size = 8,
> >> + .max_access_size = 8,
> >> + },
> >> +};
> >> +
> >> +/*
> >> * XIVE Interrupt Source
> >> */
> >> static void xive_ics_set_irq_msi(XiveICSState *xs, int srcno, int val)
> >> @@ -106,15 +326,25 @@ static void xive_ics_realize(ICSState *ics, Error
> >> **errp)
> >> return;
> >> }
> >>
> >> + if (!xs->esb_shift) {
> >> + error_setg(errp, "ESB page size needs to be greater 0");
> >> + return;
> >> + }
> >> +
> >> ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
> >> ics->qirqs = qemu_allocate_irqs(xive_ics_set_irq, xs, ics->nr_irqs);
> >>
> >> + memory_region_init_io(&xs->esb_iomem, OBJECT(xs), &xive_esb_ops, xs,
> >> + "xive.esb",
> >> + (1ull << xs->esb_shift) *
> >> ICS_BASE(xs)->nr_irqs);
> >> +
> >> qemu_register_reset(xive_ics_reset, xs);
> >> }
> >>
> >> static Property xive_ics_properties[] = {
> >> DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0),
> >> DEFINE_PROP_UINT32("irq-base", ICSState, offset, 0),
> >> + DEFINE_PROP_UINT32("shift", XiveICSState, esb_shift, 0),
> >> DEFINE_PROP_END_OF_LIST(),
> >> };
> >>
> >> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> >> index 544cc6e0c796..5303d96f5f59 100644
> >> --- a/include/hw/ppc/xive.h
> >> +++ b/include/hw/ppc/xive.h
> >> @@ -33,6 +33,9 @@ typedef struct XiveICSState XiveICSState;
> >> struct XiveICSState {
> >> ICSState parent_obj;
> >>
> >> + uint32_t esb_shift;
> >> + MemoryRegion esb_iomem;
> >> +
> >> XIVE *xive;
> >> };
> >>
> >
>
--
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
- [Qemu-devel] [RFC PATCH 06/26] ppc/xive: introduce a XIVE interrupt source model, (continued)
[Qemu-devel] [RFC PATCH 07/26] ppc/xive: add MMIO handlers to the XIVE interrupt source, Cédric Le Goater, 2017/07/05
Re: [Qemu-devel] [RFC PATCH 07/26] ppc/xive: add MMIO handlers to the XIVE interrupt source, Alexey Kardashevskiy, 2017/07/24
[Qemu-devel] [RFC PATCH 08/26] ppc/xive: add flags to the XIVE interrupt source, Cédric Le Goater, 2017/07/05
- Re: [Qemu-devel] [RFC PATCH 08/26] ppc/xive: add flags to the XIVE interrupt source, David Gibson, 2017/07/24
- Re: [Qemu-devel] [RFC PATCH 08/26] ppc/xive: add flags to the XIVE interrupt source, Benjamin Herrenschmidt, 2017/07/24
- Re: [Qemu-devel] [RFC PATCH 08/26] ppc/xive: add flags to the XIVE interrupt source, David Gibson, 2017/07/24
- Re: [Qemu-devel] [RFC PATCH 08/26] ppc/xive: add flags to the XIVE interrupt source, Benjamin Herrenschmidt, 2017/07/24
- Re: [Qemu-devel] [RFC PATCH 08/26] ppc/xive: add flags to the XIVE interrupt source, Cédric Le Goater, 2017/07/24
- Re: [Qemu-devel] [RFC PATCH 08/26] ppc/xive: add flags to the XIVE interrupt source, David Gibson, 2017/07/25