Re: [PATCH v2 2/2] x86/APIC: modify error_interrupt() to output using single printk()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.03.2023 20:53, Elliott Mitchell wrote:
> This takes care of the issue of APIC errors tending to occur on multiple
> cores at one.  In turn this tends to causes the error messages to be

Nit: "at once"?

> merged together, making understanding them difficult.
> 
> Signed-off-by: Elliott Mitchell <ehem+xen@xxxxxxx>

Here it becomes clear why you're making the change in patch 1; as you say
in the cover letter these may better be folded (or else, as said there,
patch 1 needs better justification).

> @@ -1419,12 +1420,12 @@ static void cf_check error_interrupt(struct 
> cpu_user_regs *regs)
>      v1 = apic_read(APIC_ESR);
>      ack_APIC_irq();
>  
> -    printk(XENLOG_DEBUG "APIC error on CPU%u: %02x(%02x)",
> -            smp_processor_id(), v , v1);
>      for ( i = 7; i >= 0; --i )
> -        if ( v1 & (1 << i) )
> -            printk("%s", esr_fields[i]);
> -    printk("\n");
> +        entries[i] = v1 & (1 << i) ? esr_fields[i] : "";
> +    printk(XENLOG_DEBUG "APIC error on CPU%u: %02x(%02x)"
> +            "%s%s%s%s%s%s%s%s" "\n",
> +            smp_processor_id(), v , v1, entries[0], entries[1], entries[2],
> +            entries[3], entries[4], entries[5], entries[6], entries[7]);

Two style nits: Indentation wants fixing here (it was wrong in the original
code already), and the stray blank between v and the comma also wants
dropping at this occasion.

Jan