From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42345) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WibF4-0001OA-7m for qemu-devel@nongnu.org; Thu, 08 May 2014 23:13:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WibEw-0004ed-Bi for qemu-devel@nongnu.org; Thu, 08 May 2014 23:12:58 -0400 Received: from mail-pa0-f45.google.com ([209.85.220.45]:59699) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WibEw-0004eC-5y for qemu-devel@nongnu.org; Thu, 08 May 2014 23:12:50 -0400 Received: by mail-pa0-f45.google.com with SMTP id ey11so3747653pad.18 for ; Thu, 08 May 2014 20:12:49 -0700 (PDT) Message-ID: <536C47A9.2080105@ozlabs.ru> Date: Fri, 09 May 2014 13:12:41 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1399442518-26303-1-git-send-email-aik@ozlabs.ru> <1399442518-26303-2-git-send-email-aik@ozlabs.ru> <87eh05u45p.fsf@pixel.localdomain> In-Reply-To: <87eh05u45p.fsf@pixel.localdomain> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/6] xics: add flags for interrupts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mike Day , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, Alexander Graf On 05/07/2014 11:09 PM, Mike Day wrote: > >> >> for (i = 0; i < ics->nr_irqs; i++) { >> /* FIXME: filter by server#? */ >> - if (ics->islsi[i]) { >> + if (ics->irqs[i].flags & XICS_FLAGS_LSI) { >> resend_lsi(ics, i); > > Not part of your patch, but I'm curious about this FIXME (there's an > identical FIXME in resend_msi). Has this proved to be a problem? ics_resend -> resend_lsi() -> icp_irq() and the last one delivers interrupt to the correct server. I should probably remove that FIXME or I am missing something here. > With > these patches you could have many unallocated interrupts in array AFTER > the last allocated interrupt, correct? I would have unallocated interrupts before this patch too. > In that case, the loop would > continue beyond the last allocated interrupt for no purpose. For LSI we check the type, if it is not, it is assumed MSI and then resend_msi() would check for XICS_STATUS_REJECTED which is not set for non-allocated interrupt so we are safe. But since I can tell now if it is allocated or not, I will fix it to call resend_msi() on only if irq is allocated as MSI. > There are > a couple ways to mitigate this type of situation by using alternative > data structures to inform the loop traversal. I don't know if it is > worth the effort, though. Here I lost you :) btw I just realized that in patch#2 it should be xics_find_source instead of xics_find_server. There are many interrupt servers already and just one interrupt source (we could have many like one per PHB or something like that but we are not there yet), this is what I meant. > > >> +/* @flags == 0 measn the interrupt is not allocated */ >> +#define XICS_FLAGS_LSI 0x1 >> +#define XICS_FLAGS_MSI 0x2 > > (nit) typo in the above comment > > Mike > -- Alexey