Re: [PATCH 09/15] x86/irq: Install posted MSI notification handler

Zeng Guang
2024-03-29T07:32:08Z

On 1/27/2024 7:42 AM, Jacob Pan wrote:
> @@ -353,6 +360,111 @@ void intel_posted_msi_init(void)
> pid->nv = POSTED_MSI_NOTIFICATION_VECTOR;
> pid->ndst = this_cpu_read(x86_cpu_to_apicid);
> }
> +
> +/*
> + * De-multiplexing posted interrupts is on the performance path, the code
> + * below is written to optimize the cache performance based on the following
> + * considerations:
> + * 1.Posted interrupt descriptor (PID) fits in a cache line that is frequently
> + * accessed by both CPU and IOMMU.
> + * 2.During posted MSI processing, the CPU needs to do 64-bit read and xchg
> + * for checking and clearing posted interrupt request (PIR), a 256 bit field
> + * within the PID.
> + * 3.On the other side, the IOMMU does atomic swaps of the entire PID cache
> + * line when posting interrupts and setting control bits.
> + * 4.The CPU can access the cache line a magnitude faster than the IOMMU.
> + * 5.Each time the IOMMU does interrupt posting to the PIR will evict the PID
> + * cache line. The cache line states after each operation are as follows:
> + * CPU IOMMU PID Cache line state
> + * ---------------------------------------------------------------
> + *...read64 exclusive
> + *...lock xchg64 modified
> + *... post/atomic swap invalid
> + *...-------------------------------------------------------------
> + *
> + * To reduce L1 data cache miss, it is important to avoid contention with
> + * IOMMU's interrupt posting/atomic swap. Therefore, a copy of PIR is used
> + * to dispatch interrupt handlers.
> + *
> + * In addition, the code is trying to keep the cache line state consistent
> + * as much as possible. e.g. when making a copy and clearing the PIR
> + * (assuming non-zero PIR bits are present in the entire PIR), it does:
> + * read, read, read, read, xchg, xchg, xchg, xchg
> + * instead of:
> + * read, xchg, read, xchg, read, xchg, read, xchg
> + */
> +static __always_inline inline bool handle_pending_pir(u64 *pir, struct pt_regs *regs)
> +{
> + int i, vec = FIRST_EXTERNAL_VECTOR;
> + unsigned long pir_copy[4];
> + bool handled = false;
> +
> + for (i = 0; i < 4; i++)
> + pir_copy[i] = pir[i];
> +
> + for (i = 0; i < 4; i++) {
> + if (!pir_copy[i])
> + continue;
> +
> + pir_copy[i] = arch_xchg(pir, 0);

Here is a problem that pir_copy[i] will always be written as pir[0].
This leads to handle spurious posted MSIs later.

> + handled = true;
> + }
> +
> + if (handled) {
> + for_each_set_bit_from(vec, pir_copy, FIRST_SYSTEM_VECTOR)
> + call_irq_handler(vec, regs);
> + }
> +
> + return handled;
> +}
> +
> +/*
> + * Performance data shows that 3 is good enough to harvest 90+% of the benefit
> + * on high IRQ rate workload.
> + */
> +#define MAX_POSTED_MSI_COALESCING_LOOP 3
> +
> +/*
> + * For MSIs that are delivered as posted interrupts, the CPU notifications
> + * can be coalesced if the MSIs arrive in high frequency bursts.
> + */
> +DEFINE_IDTENTRY_SYSVEC(sysvec_posted_msi_notification)
> +{
> + struct pt_regs *old_regs = set_irq_regs(regs);
> + struct pi_desc *pid;
> + int i = 0;
> +
> + pid = this_cpu_ptr(&posted_interrupt_desc);
> +
> + inc_irq_stat(posted_msi_notification_count);
> + irq_enter();
> +
> + /*
> + * Max coalescing count includes the extra round of handle_pending_pir
> + * after clearing the outstanding notification bit. Hence, at most
> + * MAX_POSTED_MSI_COALESCING_LOOP - 1 loops are executed here.
> + */
> + while (++i < MAX_POSTED_MSI_COALESCING_LOOP) {
> + if (!handle_pending_pir(pid->pir64, regs))
> + break;
> + }
> +
> + /*
> + * Clear outstanding notification bit to allow new IRQ notifications,
> + * do this last to maximize the window of interrupt coalescing.
> + */
> + pi_clear_on(pid);
> +
> + /*
> + * There could be a race of PI notification and the clearing of ON bit,
> + * process PIR bits one last time such that handling the new interrupts
> + * are not delayed until the next IRQ.
> + */
> + handle_pending_pir(pid->pir64, regs);
> +
> + apic_eoi();
> + irq_exit();
> + set_irq_regs(old_regs);
> }
> #endif /* X86_POSTED_MSI */
>