[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87zfynt6uo.ffs@tglx>
Date: Wed, 06 Dec 2023 21:26:55 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Jacob Pan <jacob.jun.pan@...ux.intel.com>,
LKML <linux-kernel@...r.kernel.org>, X86 Kernel <x86@...nel.org>,
iommu@...ts.linux.dev, Lu Baolu <baolu.lu@...ux.intel.com>,
kvm@...r.kernel.org, Dave Hansen <dave.hansen@...el.com>,
Joerg Roedel <joro@...tes.org>,
"H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
Ingo Molnar <mingo@...hat.com>
Cc: Raj Ashok <ashok.raj@...el.com>,
"Tian, Kevin" <kevin.tian@...el.com>, maz@...nel.org,
peterz@...radead.org, seanjc@...gle.com,
Robin Murphy <robin.murphy@....com>,
Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: Re: [PATCH RFC 13/13] iommu/vt-d: Enable posted mode for device MSIs
On Sat, Nov 11 2023 at 20:16, Jacob Pan wrote:
> #ifdef CONFIG_X86_POSTED_MSI
>
> static u64 get_pi_desc_addr(struct irq_data *irqd)
> @@ -1133,6 +1144,29 @@ static u64 get_pi_desc_addr(struct irq_data *irqd)
>
> return __pa(per_cpu_ptr(&posted_interrupt_desc, cpu));
> }
> +
> +static void intel_ir_reconfigure_irte_posted(struct irq_data *irqd)
> +{
> + struct intel_ir_data *ir_data = irqd->chip_data;
> + struct irte *irte = &ir_data->irte_entry;
> + struct irte irte_pi;
> + u64 pid_addr;
> +
> + pid_addr = get_pi_desc_addr(irqd);
> +
> + memset(&irte_pi, 0, sizeof(irte_pi));
> +
> + /* The shared IRTE already be set up as posted during alloc_irte */
-ENOPARSE
> + dmar_copy_shared_irte(&irte_pi, irte);
> +
> + irte_pi.pda_l = (pid_addr >> (32 - PDA_LOW_BIT)) & ~(-1UL << PDA_LOW_BIT);
> + irte_pi.pda_h = (pid_addr >> 32) & ~(-1UL << PDA_HIGH_BIT);
> +
> + modify_irte(&ir_data->irq_2_iommu, &irte_pi);
> +}
> +
> +#else
> +static inline void intel_ir_reconfigure_irte_posted(struct irq_data *irqd) {}
> #endif
>
> static void intel_ir_reconfigure_irte(struct irq_data *irqd, bool force)
> @@ -1148,8 +1182,9 @@ static void intel_ir_reconfigure_irte(struct irq_data *irqd, bool force)
> irte->vector = cfg->vector;
> irte->dest_id = IRTE_DEST(cfg->dest_apicid);
>
> - /* Update the hardware only if the interrupt is in remapped mode. */
> - if (force || ir_data->irq_2_iommu.mode == IRQ_REMAPPING)
> + if (ir_data->irq_2_iommu.posted_msi)
> + intel_ir_reconfigure_irte_posted(irqd);
> + else if (force || ir_data->irq_2_iommu.mode == IRQ_REMAPPING)
> modify_irte(&ir_data->irq_2_iommu, irte);
> }
>
> @@ -1203,7 +1238,7 @@ static int intel_ir_set_vcpu_affinity(struct irq_data *data, void *info)
> struct intel_ir_data *ir_data = data->chip_data;
> struct vcpu_data *vcpu_pi_info = info;
>
> - /* stop posting interrupts, back to remapping mode */
> + /* stop posting interrupts, back to the default mode */
> if (!vcpu_pi_info) {
> modify_irte(&ir_data->irq_2_iommu, &ir_data->irte_entry);
> } else {
> @@ -1300,10 +1335,14 @@ static void intel_irq_remapping_prepare_irte(struct intel_ir_data *data,
> {
> struct irte *irte = &data->irte_entry;
>
> - prepare_irte(irte, irq_cfg->vector, irq_cfg->dest_apicid);
> + if (data->irq_2_iommu.mode == IRQ_POSTING)
> + prepare_irte_posted(irte);
> + else
> + prepare_irte(irte, irq_cfg->vector, irq_cfg->dest_apicid);
>
> switch (info->type) {
> case X86_IRQ_ALLOC_TYPE_IOAPIC:
> + prepare_irte(irte, irq_cfg->vector, irq_cfg->dest_apicid);
What? This is just wrong. Above you have:
> + if (data->irq_2_iommu.mode == IRQ_POSTING)
> + prepare_irte_posted(irte);
> + else
> + prepare_irte(irte, irq_cfg->vector, irq_cfg->dest_apicid);
Can you spot the fail?
> /* Set source-id of interrupt request */
> set_ioapic_sid(irte, info->devid);
> apic_printk(APIC_VERBOSE, KERN_DEBUG "IOAPIC[%d]: Set IRTE entry (P:%d FPD:%d Dst_Mode:%d Redir_hint:%d Trig_Mode:%d Dlvry_Mode:%X Avail:%X Vector:%02X Dest:%08X SID:%04X SQ:%X SVT:%X)\n",
> @@ -1315,10 +1354,18 @@ static void intel_irq_remapping_prepare_irte(struct intel_ir_data *data,
> sub_handle = info->ioapic.pin;
> break;
> case X86_IRQ_ALLOC_TYPE_HPET:
> + prepare_irte(irte, irq_cfg->vector, irq_cfg->dest_apicid);
> set_hpet_sid(irte, info->devid);
> break;
> case X86_IRQ_ALLOC_TYPE_PCI_MSI:
> case X86_IRQ_ALLOC_TYPE_PCI_MSIX:
> + if (posted_msi_supported()) {
> + prepare_irte_posted(irte);
> + data->irq_2_iommu.posted_msi = 1;
> + } else {
> + prepare_irte(irte, irq_cfg->vector, irq_cfg->dest_apicid);
> + }
Here it gets even more hilarious.
Powered by blists - more mailing lists