[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 39/57] ARM: new VGIC: Add ACTIVE registers handlers
Hi, On 13/03/18 17:42, Julien Grall wrote: > Hi, > > On 13/03/18 17:34, Andre Przywara wrote: >> On 13/03/18 17:14, Julien Grall wrote: >>> On 13/03/18 17:02, Andre Przywara wrote: >>>> On 08/03/18 15:39, Julien Grall wrote: >>>>> On 05/03/18 16:03, Andre Przywara wrote: >>> I can't see how a knowledgeable admin will be able to know that IRQ 2 is >>> active with just the register value. >> >> Well, I was assuming that a really knowledgeable admin would somehow >> forward the error message either to the ML or at least to $search_engine. >> ... > > Surely, but it does not mean the message should be clueless for the > developer. I would rather no spent 10 min to try to find out what's > going on where reading logs... > >> >>>> Does that sound OK? >>> >>> I would still prefer the one per IRQ and using printk(XENLOG_G_*). >> >> I really don't think one per IRQ is too useful. A developer however can >> easily deal with "IRQ45, value: 0x00802000" from a log. And can deduce >> from there that it's about IRQ45 and IRQ55. Following the example above >> you would either see one "IRQ32, value: 0xffffffff" or "IRQ 45, value: >> 0x00002000". > > I still can't see how the developer would know the IRQ55 is active or > not. That's the whole purpose of the per IRQ. > >> That looks like a good compromise between readability (having the IRQ >> number for admins) and brevity. > > You may save 10 characters on the logs, you likely going to waste 10 min > of the developer to understand what that messages really mean. It's not about 10 characters, it's about 31 *lines*. At the moment a write to I[CS]ACTIVER triggers *one* line in the log: %pv: vGICD: unhandled word write %#"PRIregister" to ISACTIVER%d Now a write to this register would potentially trigger *32* lines: %pv: vGICD: IRQ%u: setting active state not supported By dumping the line as I proposed, I basically mimic the current line, plus give some information about one IRQ affected: %pv: vGICD: setting active state not supported (IRQ%u, value: 0x%08lx) So this is not a regression, but an improvement. Cheers, Andre. >> I changed it now to output: >> %pv: vGICD: clearing active state not supported (IRQ%u, value: 0x%08lx) >> >>> I don't much care about the spam, see why above. >> >> Having them on the console between Dom0 messages is really scary, but >> not helpful if it's *more* than one. Since it's a known limitation of >> the VGIC emulation, not a real "error" in that sense. > > It is the same as having any Xen messages interleaved with Dom0 > messages. If the user is not happy with that, then it can divert Dom0 > console to another UART. > > Cheers, > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |