[PATCH v2 0/2] PCI: vmd: IRQ domain assignment to sub devices

[PATCH v2 0/2] PCI: vmd: IRQ domain assignment to sub devices

[Nirmal Patel]

Host OS fails to boot and DMAR errors were observed when interrupt
remapping is enabled by intel_iommu because of the fact that VMD child
devices are on different IRQ domain than all other PCI devices.
Make sure VMD assigns proper IRQ domain to the child devices during
device enumeration.

Nirmal Patel (2):
  PCI: vmd: Assign VMD IRQ domain before enumeration
  PCI: vmd: Revert 2565e5b69c44 ("PCI: vmd: Do not disable MSI-X
    remapping if interrupt remapping is enabled by IOMMU.")

 drivers/pci/controller/vmd.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

-- 
2.26.2

[PATCH v2 1/2] PCI: vmd: Assign VMD IRQ domain before enumeration

[Nirmal Patel]

During the boot process all the PCI devices are assigned default PCI-MSI
IRQ domain including VMD endpoint devices. If interrupt-remapping is
enabled by IOMMU, the PCI devices except VMD get new INTEL-IR-MSI IRQ
domain. And VMD is supposed to create and assign a separate VMD-MSI IRQ
domain for its child devices in order to support MSI-X remapping
capabilities.

Now when MSI-X remapping in VMD is disabled in order to improve
performance, VMD skips VMD-MSI IRQ domain assignment process to its
child devices. Thus the devices behind VMD get default PCI-MSI IRQ
domain instead of INTEL-IR-MSI IRQ domain when VMD creates root bus and
configures child devices.

As a result host OS fails to boot and DMAR errors were observed when
interrupt remapping was enabled on Intel Icelake CPUs. For instance:

  DMAR: DRHD: handling fault status reg 2
  DMAR: [INTR-REMAP] Request device [0xe2:0x00.0] fault index 0xa00 [fault reason 0x25] Blocked a compatibility format interrupt request

To fix this issue, dev_msi_info struct in dev struct maintains correct
value of IRQ domain. VMD will use this information to assign proper IRQ
domain to its child devices when it doesn't create a separate IRQ domain.

Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
---
v1->v2: Adding more information to commit log.
---
 drivers/pci/controller/vmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index eb05cceab964..5015adc04d19 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -853,6 +853,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	vmd_attach_resources(vmd);
 	if (vmd->irq_domain)
 		dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
+	else
+		dev_set_msi_domain(&vmd->bus->dev,
+				   dev_get_msi_domain(&vmd->dev->dev));
 
 	vmd_acpi_begin();
 
-- 
2.26.2

[PATCH v2 2/2] PCI: vmd: Revert 2565e5b69c44 ("PCI: vmd: Do not disable MSI-X remapping if interrupt remapping is enabled by IOMMU.")

[Nirmal Patel]

Revert 2565e5b69c44 ("PCI: vmd: Do not disable MSI-X remapping if
interrupt remapping is enabled by IOMMU.")

The commit 2565e5b69c44 was added as a workaround to keep MSI-X
remapping enabled if IOMMU enables interrupt remapping. VMD would keep
running in low performance mode. There is no dependency between MSI-X
remapping by VMD and interrupt remapping by IOMMU.

Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
---
v1->v2: Add more information to the commit log.
---
 drivers/pci/controller/vmd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 5015adc04d19..94a14a3d7e55 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -6,7 +6,6 @@
 
 #include <linux/device.h>
 #include <linux/interrupt.h>
-#include <linux/iommu.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -813,8 +812,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	 * acceptable because the guest is usually CPU-limited and MSI
 	 * remapping doesn't become a performance bottleneck.
 	 */
-	if (iommu_capable(vmd->dev->dev.bus, IOMMU_CAP_INTR_REMAP) ||
-	    !(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
+	if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
 	    offset[0] || offset[1]) {
 		ret = vmd_alloc_irqs(vmd);
 		if (ret)
-- 
2.26.2

Re: [PATCH v2 0/2] PCI: vmd: IRQ domain assignment to sub devices

[Lorenzo Pieralisi]

On Wed, 11 May 2022 02:57:05 -0700, Nirmal Patel wrote:
> Host OS fails to boot and DMAR errors were observed when interrupt
> remapping is enabled by intel_iommu because of the fact that VMD child
> devices are on different IRQ domain than all other PCI devices.
> Make sure VMD assigns proper IRQ domain to the child devices during
> device enumeration.
> 
> Nirmal Patel (2):
>   PCI: vmd: Assign VMD IRQ domain before enumeration
>   PCI: vmd: Revert 2565e5b69c44 ("PCI: vmd: Do not disable MSI-X
>     remapping if interrupt remapping is enabled by IOMMU.")
> 
> [...]

Applied to pci/vmd, thanks!

[1/2] PCI: vmd: Assign VMD IRQ domain before enumeration
      https://git.kernel.org/lpieralisi/pci/c/886e67100b
[2/2] PCI: vmd: Revert 2565e5b69c44 ("PCI: vmd: Do not disable MSI-X remapping if interrupt remapping is enabled by IOMMU.")
      https://git.kernel.org/lpieralisi/pci/c/c94f732e80

Thanks,
Lorenzo

Re: [PATCH v2 1/2] PCI: vmd: Assign VMD IRQ domain before enumeration

[Bjorn Helgaas]

On Wed, Mar 30, 2022 at 11:06:43AM -0700, Patel, Nirmal wrote:
> On 3/30/2022 10:54 AM, Bjorn Helgaas wrote:
> ...

> > This claims to be a v2, but I missed the v1, and the lore archives [1]
> > seem incomplete.  Maybe the v1 (and maybe the cover letter?) were HTML
> > or got lost for some other reason?
> >
> > Bjorn
> >
> > [1] https://lore.kernel.org/all/?q=f%3Anirmal.patel
> 
> Initially I created one patch [1] and I was advised to create two separate patches.
> 
> [1] https://lore.kernel.org/all/358b0673-f90f-78ca-be66-51d5f76cc42b@linux.intel.com/

The point is that
https://lore.kernel.org/all/358b0673-f90f-78ca-be66-51d5f76cc42b@linux.intel.com/
is a reply to something that didn't make it into the archives.

It says:

  In-Reply-To: <20220223075245.17744-1-nirmal.patel@linux.intel.com>

but 20220223075245.17744-1-nirmal.patel@linux.intel.com is unknown to
lore.

You can't fix this now; I'm just pointing it out in case there's some
problem with the way you're sending patches.

Bjorn

Re: [PATCH v2 1/2] PCI: vmd: Assign VMD IRQ domain before enumeration

[Patel, Nirmal]

On 3/30/2022 10:54 AM, Bjorn Helgaas wrote:
> On Wed, Mar 30, 2022 at 08:54:15AM -0700, Patel, Nirmal wrote:
>> On 3/29/2022 4:27 PM, Dan Williams wrote:
>>> On Tue, Mar 29, 2022 at 3:48 PM Patel, Nirmal
>>> <nirmal.patel@linux.intel.com> wrote:
>>>> On 3/16/2022 8:51 AM, Nirmal Patel wrote:
>>>>> From: Nirmal Patel <nirmal.patel@linux.intel.com>
>>>>>
>>>>> VMD creates and assigns a separate IRQ domain only when MSI remapping is
>>>>> enabled. For example VMD-MSI. But VMD doesn't assign IRQ domain when
>>>>> MSI remapping is disabled resulting child devices getting default
>>>>> PCI-MSI IRQ domain. Now when interrupt remapping is enabled by
>>>>> intel-iommu all the PCI devices are assigned INTEL-IR-MSI domain
>>>>> including VMD endpoints. But devices behind VMD get PCI-MSI IRQ domain
>>>>> when VMD create a root bus and configures child devices.
>>>>>
>>>>> As a result DMAR errors were observed when interrupt remapping was
>>>>> enabled on Intel Icelake CPUs. For instance:
>>>>>
>>>>>   DMAR: DRHD: handling fault status reg 2
>>>>>   DMAR: [INTR-REMAP] Request device [0xe2:0x00.0] fault index 0xa00 [fault reason 0x25] Blocked a compatibility format interrupt request
>>>>>
>>>>> Acked-by: Dan Williams <dan.j.williams@intel.com>
>>>>> Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
>>>>> ---
>>>>>  drivers/pci/controller/vmd.c | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
>>>>> index cc166c683638..3a6570e5b765 100644
>>>>> --- a/drivers/pci/controller/vmd.c
>>>>> +++ b/drivers/pci/controller/vmd.c
>>>>> @@ -853,6 +853,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>>>>>       vmd_attach_resources(vmd);
>>>>>       if (vmd->irq_domain)
>>>>>               dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
>>>>> +     else
>>>>> +             dev_set_msi_domain(&vmd->bus->dev, dev_get_msi_domain(&vmd->dev->dev));
>>>>>
>>>>>       vmd_acpi_begin();
>>>>>
>>>> Gentle ping!
>>> It helps to be explicit when you send a patch and a follow-up ping.
>>> Are you asking Lorenzo to take this? Is this urgent such that Bjorn
>>> should consider taking it directly? The changelog notes what happens,
>>> but not the severity of end user visible impact. The merge window is
>>> presently open so the natural inclination is to just wait until that
>>> closes to circle back to outstanding patches.
>> This patch removes a flag that bypasses MSI disable feature of VMD and
>> improves the performance. So it would be nice if the patch gets accepted
>> sooner. I tend to send follow-up ping after a week or so if I do not get any
>> feedback and to allow it to get accepted in time.
> There are only a few days left in the v5.18 merge window, so unless
> it's an emergency, this would be v5.19 material.
>
> This claims to be a v2, but I missed the v1, and the lore archives [1]
> seem incomplete.  Maybe the v1 (and maybe the cover letter?) were HTML
> or got lost for some other reason?
>
> Bjorn
>
> [1] https://lore.kernel.org/all/?q=f%3Anirmal.patel

Initially I created one patch [1] and I was advised to create two separate patches.

[1] https://lore.kernel.org/all/358b0673-f90f-78ca-be66-51d5f76cc42b@linux.intel.com/

Re: [PATCH v2 1/2] PCI: vmd: Assign VMD IRQ domain before enumeration

[Bjorn Helgaas]

On Wed, Mar 30, 2022 at 08:54:15AM -0700, Patel, Nirmal wrote:
> On 3/29/2022 4:27 PM, Dan Williams wrote:
> > On Tue, Mar 29, 2022 at 3:48 PM Patel, Nirmal
> > <nirmal.patel@linux.intel.com> wrote:
> >> On 3/16/2022 8:51 AM, Nirmal Patel wrote:
> >>> From: Nirmal Patel <nirmal.patel@linux.intel.com>
> >>>
> >>> VMD creates and assigns a separate IRQ domain only when MSI remapping is
> >>> enabled. For example VMD-MSI. But VMD doesn't assign IRQ domain when
> >>> MSI remapping is disabled resulting child devices getting default
> >>> PCI-MSI IRQ domain. Now when interrupt remapping is enabled by
> >>> intel-iommu all the PCI devices are assigned INTEL-IR-MSI domain
> >>> including VMD endpoints. But devices behind VMD get PCI-MSI IRQ domain
> >>> when VMD create a root bus and configures child devices.
> >>>
> >>> As a result DMAR errors were observed when interrupt remapping was
> >>> enabled on Intel Icelake CPUs. For instance:
> >>>
> >>>   DMAR: DRHD: handling fault status reg 2
> >>>   DMAR: [INTR-REMAP] Request device [0xe2:0x00.0] fault index 0xa00 [fault reason 0x25] Blocked a compatibility format interrupt request
> >>>
> >>> Acked-by: Dan Williams <dan.j.williams@intel.com>
> >>> Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> >>> ---
> >>>  drivers/pci/controller/vmd.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> >>> index cc166c683638..3a6570e5b765 100644
> >>> --- a/drivers/pci/controller/vmd.c
> >>> +++ b/drivers/pci/controller/vmd.c
> >>> @@ -853,6 +853,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
> >>>       vmd_attach_resources(vmd);
> >>>       if (vmd->irq_domain)
> >>>               dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
> >>> +     else
> >>> +             dev_set_msi_domain(&vmd->bus->dev, dev_get_msi_domain(&vmd->dev->dev));
> >>>
> >>>       vmd_acpi_begin();
> >>>
> >> Gentle ping!
> > It helps to be explicit when you send a patch and a follow-up ping.
> > Are you asking Lorenzo to take this? Is this urgent such that Bjorn
> > should consider taking it directly? The changelog notes what happens,
> > but not the severity of end user visible impact. The merge window is
> > presently open so the natural inclination is to just wait until that
> > closes to circle back to outstanding patches.
> 
> This patch removes a flag that bypasses MSI disable feature of VMD and
> improves the performance. So it would be nice if the patch gets accepted
> sooner. I tend to send follow-up ping after a week or so if I do not get any
> feedback and to allow it to get accepted in time.

There are only a few days left in the v5.18 merge window, so unless
it's an emergency, this would be v5.19 material.

This claims to be a v2, but I missed the v1, and the lore archives [1]
seem incomplete.  Maybe the v1 (and maybe the cover letter?) were HTML
or got lost for some other reason?

Bjorn

[1] https://lore.kernel.org/all/?q=f%3Anirmal.patel

Re: [PATCH v2 1/2] PCI: vmd: Assign VMD IRQ domain before enumeration

[Patel, Nirmal]

On 3/29/2022 4:27 PM, Dan Williams wrote:
> On Tue, Mar 29, 2022 at 3:48 PM Patel, Nirmal
> <nirmal.patel@linux.intel.com> wrote:
>> On 3/16/2022 8:51 AM, Nirmal Patel wrote:
>>> From: Nirmal Patel <nirmal.patel@linux.intel.com>
>>>
>>> VMD creates and assigns a separate IRQ domain only when MSI remapping is
>>> enabled. For example VMD-MSI. But VMD doesn't assign IRQ domain when
>>> MSI remapping is disabled resulting child devices getting default
>>> PCI-MSI IRQ domain. Now when interrupt remapping is enabled by
>>> intel-iommu all the PCI devices are assigned INTEL-IR-MSI domain
>>> including VMD endpoints. But devices behind VMD get PCI-MSI IRQ domain
>>> when VMD create a root bus and configures child devices.
>>>
>>> As a result DMAR errors were observed when interrupt remapping was
>>> enabled on Intel Icelake CPUs. For instance:
>>>
>>>   DMAR: DRHD: handling fault status reg 2
>>>   DMAR: [INTR-REMAP] Request device [0xe2:0x00.0] fault index 0xa00 [fault reason 0x25] Blocked a compatibility format interrupt request
>>>
>>> Acked-by: Dan Williams <dan.j.williams@intel.com>
>>> Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
>>> ---
>>>  drivers/pci/controller/vmd.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
>>> index cc166c683638..3a6570e5b765 100644
>>> --- a/drivers/pci/controller/vmd.c
>>> +++ b/drivers/pci/controller/vmd.c
>>> @@ -853,6 +853,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>>>       vmd_attach_resources(vmd);
>>>       if (vmd->irq_domain)
>>>               dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
>>> +     else
>>> +             dev_set_msi_domain(&vmd->bus->dev, dev_get_msi_domain(&vmd->dev->dev));
>>>
>>>       vmd_acpi_begin();
>>>
>> Gentle ping!
> It helps to be explicit when you send a patch and a follow-up ping.
> Are you asking Lorenzo to take this? Is this urgent such that Bjorn
> should consider taking it directly? The changelog notes what happens,
> but not the severity of end user visible impact. The merge window is
> presently open so the natural inclination is to just wait until that
> closes to circle back to outstanding patches.

This patch removes a flag that bypasses MSI disable feature of VMD and
improves the performance. So it would be nice if the patch gets accepted
sooner. I tend to send follow-up ping after a week or so if I do not get any
feedback and to allow it to get accepted in time.

Thanks.

Re: [PATCH v2 1/2] PCI: vmd: Assign VMD IRQ domain before enumeration

[Dan Williams]

On Tue, Mar 29, 2022 at 3:48 PM Patel, Nirmal
<nirmal.patel@linux.intel.com> wrote:
>
> On 3/16/2022 8:51 AM, Nirmal Patel wrote:
> > From: Nirmal Patel <nirmal.patel@linux.intel.com>
> >
> > VMD creates and assigns a separate IRQ domain only when MSI remapping is
> > enabled. For example VMD-MSI. But VMD doesn't assign IRQ domain when
> > MSI remapping is disabled resulting child devices getting default
> > PCI-MSI IRQ domain. Now when interrupt remapping is enabled by
> > intel-iommu all the PCI devices are assigned INTEL-IR-MSI domain
> > including VMD endpoints. But devices behind VMD get PCI-MSI IRQ domain
> > when VMD create a root bus and configures child devices.
> >
> > As a result DMAR errors were observed when interrupt remapping was
> > enabled on Intel Icelake CPUs. For instance:
> >
> >   DMAR: DRHD: handling fault status reg 2
> >   DMAR: [INTR-REMAP] Request device [0xe2:0x00.0] fault index 0xa00 [fault reason 0x25] Blocked a compatibility format interrupt request
> >
> > Acked-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> > ---
> >  drivers/pci/controller/vmd.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > index cc166c683638..3a6570e5b765 100644
> > --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c
> > @@ -853,6 +853,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
> >       vmd_attach_resources(vmd);
> >       if (vmd->irq_domain)
> >               dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
> > +     else
> > +             dev_set_msi_domain(&vmd->bus->dev, dev_get_msi_domain(&vmd->dev->dev));
> >
> >       vmd_acpi_begin();
> >
>
> Gentle ping!

It helps to be explicit when you send a patch and a follow-up ping.
Are you asking Lorenzo to take this? Is this urgent such that Bjorn
should consider taking it directly? The changelog notes what happens,
but not the severity of end user visible impact. The merge window is
presently open so the natural inclination is to just wait until that
closes to circle back to outstanding patches.

Re: [PATCH v2 1/2] PCI: vmd: Assign VMD IRQ domain before enumeration

[Patel, Nirmal]

On 3/16/2022 8:51 AM, Nirmal Patel wrote:
> From: Nirmal Patel <nirmal.patel@linux.intel.com>
>
> VMD creates and assigns a separate IRQ domain only when MSI remapping is
> enabled. For example VMD-MSI. But VMD doesn't assign IRQ domain when
> MSI remapping is disabled resulting child devices getting default
> PCI-MSI IRQ domain. Now when interrupt remapping is enabled by
> intel-iommu all the PCI devices are assigned INTEL-IR-MSI domain
> including VMD endpoints. But devices behind VMD get PCI-MSI IRQ domain
> when VMD create a root bus and configures child devices.
>
> As a result DMAR errors were observed when interrupt remapping was
> enabled on Intel Icelake CPUs. For instance:
>
>   DMAR: DRHD: handling fault status reg 2
>   DMAR: [INTR-REMAP] Request device [0xe2:0x00.0] fault index 0xa00 [fault reason 0x25] Blocked a compatibility format interrupt request
>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> ---
>  drivers/pci/controller/vmd.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index cc166c683638..3a6570e5b765 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -853,6 +853,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  	vmd_attach_resources(vmd);
>  	if (vmd->irq_domain)
>  		dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
> +	else
> +		dev_set_msi_domain(&vmd->bus->dev, dev_get_msi_domain(&vmd->dev->dev));
>  
>  	vmd_acpi_begin();
>  

Gentle ping!

Thanks
nirmal