Hi Cédric, On 9/24/19 11:55 AM, Cédric Le Goater wrote: > On 24/09/2019 09:52, Greg Kurz wrote: >> On Tue, 24 Sep 2019 07:22:51 +0200 >> Cédric Le Goater <c...@kaod.org> wrote: >> >>> On 24/09/2019 06:59, David Gibson wrote: >>>> The XICS interrupt controller device used to have separate subtypes >>>> for the KVM and non-KVM variant of the device. That was a bad idea, >>>> because it leaked information that should be entirely host-side >>>> implementation specific to the kinda-sorta guest visible QOM class >>>> names. >>>> >>>> We eliminated the KVM specific class some time ago, but it's left >>>> behind a distinction between the TYPE_ICS_BASE abstract class and >>>> TYPE_ICS_SIMPLE subtype which no longer serves any purpose. >>>> >>>> This series collapses the two types back into one. >>> >>> OK. Is it migration compatible ? because this is why we have kept >> >> Yeah, the types themselves don't matter, only the format of the >> binary stream described in the VMStateDescription does. >> >>> this subclass. I am glad we can remove it finally. >>> >> >> I was thinking we were keeping that for pnv... > > > Yes, also. See the resend and reject handler in the code > below. > > > I have been maintaining this patch since QEMU 2.6. I think > it is time for me to declare defeat on getting it merged > and QEMU 4.2 will be the last rebase.
Do you remember what is missing for being merged upstream? > /* > * QEMU PowerPC PowerNV PHB3 model > * > * Copyright (c) 2014-2018, IBM Corporation. > * > * This code is licensed under the GPL version 2 or later. See the > * COPYING file in the top-level directory. > */ > #include "qemu/osdep.h" > #include "qemu/log.h" > #include "qapi/error.h" > #include "qemu-common.h" > #include "hw/pci-host/pnv_phb3_regs.h" > #include "hw/pci-host/pnv_phb3.h" > #include "hw/ppc/pnv.h" > #include "hw/pci/msi.h" > #include "monitor/monitor.h" > #include "hw/irq.h" > #include "hw/qdev-properties.h" > #include "sysemu/reset.h" > > static uint64_t phb3_msi_ive_addr(PnvPHB3 *phb, int srcno) > { > uint64_t ivtbar = phb->regs[PHB_IVT_BAR >> 3]; > uint64_t phbctl = phb->regs[PHB_CONTROL >> 3]; > > if (!(ivtbar & PHB_IVT_BAR_ENABLE)) { > qemu_log_mask(LOG_GUEST_ERROR, "Failed access to disable IVT BAR !"); > return 0; > } > > if (srcno >= (ivtbar & PHB_IVT_LENGTH_MASK)) { > qemu_log_mask(LOG_GUEST_ERROR, "MSI out of bounds (%d vs > 0x%"PRIx64")", > srcno, (uint64_t) (ivtbar & PHB_IVT_LENGTH_MASK)); > return 0; > } > > ivtbar &= PHB_IVT_BASE_ADDRESS_MASK; > > if (phbctl & PHB_CTRL_IVE_128_BYTES) { > return ivtbar + 128 * srcno; > } else { > return ivtbar + 16 * srcno; > } > } > > static bool phb3_msi_read_ive(PnvPHB3 *phb, int srcno, uint64_t *out_ive) > { > uint64_t ive_addr, ive; > > ive_addr = phb3_msi_ive_addr(phb, srcno); > if (!ive_addr) { > return false; > } > > if (dma_memory_read(&address_space_memory, ive_addr, &ive, sizeof(ive))) { > qemu_log_mask(LOG_GUEST_ERROR, "Failed to read IVE at 0x%" PRIx64, > ive_addr); > return false; > } > *out_ive = be64_to_cpu(ive); > > return true; > } > > static void phb3_msi_set_p(Phb3MsiState *msi, int srcno, uint8_t gen) > { > uint64_t ive_addr; > uint8_t p = 0x01 | (gen << 1); > > ive_addr = phb3_msi_ive_addr(msi->phb, srcno); > if (!ive_addr) { > return; > } > > if (dma_memory_write(&address_space_memory, ive_addr + 4, &p, 1)) { > qemu_log_mask(LOG_GUEST_ERROR, > "Failed to write IVE (set P) at 0x%" PRIx64, ive_addr); > } > } > > static void phb3_msi_set_q(Phb3MsiState *msi, int srcno) > { > uint64_t ive_addr; > uint8_t q = 0x01; > > ive_addr = phb3_msi_ive_addr(msi->phb, srcno); > if (!ive_addr) { > return; > } > > if (dma_memory_write(&address_space_memory, ive_addr + 5, &q, 1)) { > qemu_log_mask(LOG_GUEST_ERROR, > "Failed to write IVE (set Q) at 0x%" PRIx64, ive_addr); > } > } > > static void phb3_msi_try_send(Phb3MsiState *msi, int srcno, bool force) > { > ICSState *ics = ICS_BASE(msi); > uint64_t ive; > uint64_t server, prio, pq, gen; > > if (!phb3_msi_read_ive(msi->phb, srcno, &ive)) { > return; > } > > server = GETFIELD(IODA2_IVT_SERVER, ive); > prio = GETFIELD(IODA2_IVT_PRIORITY, ive); > if (!force) { > pq = GETFIELD(IODA2_IVT_Q, ive) | (GETFIELD(IODA2_IVT_P, ive) << 1); > } else { > pq = 0; > } > gen = GETFIELD(IODA2_IVT_GEN, ive); > > /* > * The low order 2 bits are the link pointer (Type II interrupts). > * Shift back to get a valid IRQ server. > */ > server >>= 2; > > switch (pq) { > case 0: /* 00 */ > if (prio == 0xff) { > /* Masked, set Q */ > phb3_msi_set_q(msi, srcno); > } else { > /* Enabled, set P and send */ > phb3_msi_set_p(msi, srcno, gen); > icp_irq(ics, server, srcno + ics->offset, prio); > } > break; > case 2: /* 10 */ > /* Already pending, set Q */ > phb3_msi_set_q(msi, srcno); > break; > case 1: /* 01 */ > case 3: /* 11 */ > default: > /* Just drop stuff if Q already set */ > break; > } > } > > static void phb3_msi_set_irq(void *opaque, int srcno, int val) > { > Phb3MsiState *msi = PHB3_MSI(opaque); > > if (val) { > phb3_msi_try_send(msi, srcno, false); > } > } > > > void pnv_phb3_msi_send(Phb3MsiState *msi, uint64_t addr, uint16_t data, > int32_t dev_pe) > { > ICSState *ics = ICS_BASE(msi); > uint64_t ive; > uint16_t pe; > uint32_t src = ((addr >> 4) & 0xffff) | (data & 0x1f); > > if (src >= ics->nr_irqs) { > qemu_log_mask(LOG_GUEST_ERROR, "MSI %d out of bounds", src); > return; > } > if (dev_pe >= 0) { > if (!phb3_msi_read_ive(msi->phb, src, &ive)) { > return; > } > pe = GETFIELD(IODA2_IVT_PE, ive); > if (pe != dev_pe) { > qemu_log_mask(LOG_GUEST_ERROR, > "MSI %d send by PE#%d but assigned to PE#%d", > src, dev_pe, pe); > return; > } > } > qemu_irq_pulse(msi->qirqs[src]); > } > > void pnv_phb3_msi_ffi(Phb3MsiState *msi, uint64_t val) > { > /* Emit interrupt */ > pnv_phb3_msi_send(msi, val, 0, -1); > > /* Clear FFI lock */ > msi->phb->regs[PHB_FFI_LOCK >> 3] = 0; > } > > static void phb3_msi_reject(ICSState *ics, uint32_t nr) > { > Phb3MsiState *msi = PHB3_MSI(ics); > unsigned int srcno = nr - ics->offset; > unsigned int idx = srcno >> 6; > unsigned int bit = 1ull << (srcno & 0x3f); > > assert(srcno < PHB3_MAX_MSI); > > msi->rba[idx] |= bit; > msi->rba_sum |= (1u << idx); > } > > static void phb3_msi_resend(ICSState *ics) > { > Phb3MsiState *msi = PHB3_MSI(ics); > unsigned int i, j; > > if (msi->rba_sum == 0) { > return; > } > > for (i = 0; i < 32; i++) { > if ((msi->rba_sum & (1u << i)) == 0) { > continue; > } > msi->rba_sum &= ~(1u << i); > for (j = 0; j < 64; j++) { > if ((msi->rba[i] & (1ull << j)) == 0) { > continue; > } > msi->rba[i] &= ~(1u << j); > phb3_msi_try_send(msi, i * 64 + j, true); > } > } > } > > static void phb3_msi_print_info(ICSState *ics, Monitor *mon) > { > Phb3MsiState *msi = PHB3_MSI(ics); > int i; > > for (i = 0; i < ics->nr_irqs; i++) { > uint64_t ive; > > if (!phb3_msi_read_ive(msi->phb, i, &ive)) { > return; > } > > if (GETFIELD(IODA2_IVT_PRIORITY, ive) == 0xff) { > continue; > } > > monitor_printf(mon, " %4x %c%c server=%04x prio=%02x gen=%d\n", > ics->offset + i, > GETFIELD(IODA2_IVT_P, ive) ? 'P' : '-', > GETFIELD(IODA2_IVT_Q, ive) ? 'Q' : '-', > (uint32_t) GETFIELD(IODA2_IVT_SERVER, ive) >> 2, > (uint32_t) GETFIELD(IODA2_IVT_PRIORITY, ive), > (uint32_t) GETFIELD(IODA2_IVT_GEN, ive)); > } > } > > static void phb3_msi_reset(DeviceState *dev) > { > Phb3MsiState *msi = PHB3_MSI(dev); > ICSStateClass *icsc = ICS_BASE_GET_CLASS(dev); > > icsc->parent_reset(dev); > > memset(msi->rba, 0, sizeof(msi->rba)); > msi->rba_sum = 0; > } > > static void phb3_msi_reset_handler(void *dev) > { > phb3_msi_reset(dev); > } > > void pnv_phb3_msi_update_config(Phb3MsiState *msi, uint32_t base, > uint32_t count) > { > ICSState *ics = ICS_BASE(msi); > > if (count > PHB3_MAX_MSI) { > count = PHB3_MAX_MSI; > } > ics->nr_irqs = count; > ics->offset = base; > } > > static void phb3_msi_realize(DeviceState *dev, Error **errp) > { > Phb3MsiState *msi = PHB3_MSI(dev); > ICSState *ics = ICS_BASE(msi); > ICSStateClass *icsc = ICS_BASE_GET_CLASS(ics); > Object *obj; > Error *local_err = NULL; > > icsc->parent_realize(dev, &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > } > > obj = object_property_get_link(OBJECT(dev), "phb", &local_err); > if (!obj) { > error_propagate(errp, local_err); > error_prepend(errp, "required link 'phb' not found: "); > return; > } > msi->phb = PNV_PHB3(obj); > > msi->qirqs = qemu_allocate_irqs(phb3_msi_set_irq, msi, ics->nr_irqs); > > qemu_register_reset(phb3_msi_reset_handler, dev); > } > > static void phb3_msi_instance_init(Object *obj) > { > ICSState *ics = ICS_BASE(obj); > > /* Will be overriden later */ > ics->offset = 0; > } > > static void phb3_msi_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > ICSStateClass *isc = ICS_BASE_CLASS(klass); > > device_class_set_parent_realize(dc, phb3_msi_realize, > &isc->parent_realize); > device_class_set_parent_reset(dc, phb3_msi_reset, > &isc->parent_reset); > > isc->reject = phb3_msi_reject; > isc->resend = phb3_msi_resend; > isc->print_info = phb3_msi_print_info; > } > > static const TypeInfo phb3_msi_info = { > .name = TYPE_PHB3_MSI, > .parent = TYPE_ICS_BASE, > .instance_size = sizeof(Phb3MsiState), > .class_init = phb3_msi_class_init, > .class_size = sizeof(ICSStateClass), > .instance_init = phb3_msi_instance_init, > }; > > static void pnv_phb3_msi_register_types(void) > { > type_register_static(&phb3_msi_info); > } > > type_init(pnv_phb3_msi_register_types) >