On Mon, Nov 07, 2022 at 10:27:35AM -0800, Francisco@xxxxxxxxxxxxxxx wrote:
> From: Nirmal Patel <nirmal.patel@xxxxxxxxxxxxxxx>
>
> MSI remapping is disbaled by VMD driver for intel's icelake and
disabled
Intel's
Ice Lake (at least per intel.com)
> newer systems in order to improve performance by setting MSI_RMP_DIS
> bit. By design MSI_RMP_DIS bit of VMCONFIG registers is cleared.
register is
"MSI_RMP_DIS" doesn't appear in the kernel source. Is it the same as
VMCONFIG_MSI_REMAP?
> The same register gets cleared when system is put in S3 power state.
> VMD driver needs to set this register again in order to avoid
> interrupt issues with devices behind VMD if MSI remapping was
> disabled before.
Should there be a Fixes: tag here? I guess the failure symptom is
that a driver doesn't see interrupts it expects?
> Signed-off-by: Nirmal Patel <nirmal.patel@xxxxxxxxxxxxxxx>
> Reviewed-by: Francisco Munoz <francisco.munoz.ruiz@xxxxxxxxxxxxxxx>
> ---
> 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 e06e9f4fc50f..247012b343fd 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -980,6 +980,9 @@ static int vmd_resume(struct device *dev)
> struct vmd_dev *vmd = pci_get_drvdata(pdev);
> int err, i;
>
> + if (!vmd->irq_domain)
> + vmd_set_msi_remapping(vmd, false);
I suppose this is functionally equivalent to this code in
vmd_enable_domain():
if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
offset[0] || offset[1]) {
ret = vmd_create_irq_domain(vmd);
} else {
vmd_set_msi_remapping(vmd, false);
because vmd->irq_domain will be NULL if we disabled MSI remapping at
probe-time.
Maybe the vmd_enable_domain() code could be rearranged a little bit to
make it obvious that we're doing the same thing both at probe-time and
resume-time?
Should the vmd_resume() code *enable* MSI remapping in the other case,
e.g.,
if (vmd->irq_domain)
vmd_set_msi_remapping(vmd, true);
else
vmd_set_msi_remapping(vmd, false);
I don't know what clears PCI_REG_VMCONFIG (which enabled MSI remapping
IIUC). If it's cleared by firmware, the current patch depends on that
firmware behavior, so maybe it would be good to remove that
dependency?
> for (i = 0; i < vmd->msix_count; i++) {
> err = devm_request_irq(dev, vmd->irqs[i].virq,
> vmd_irq, IRQF_NO_THREAD,
> --
> 2.34.1
>
