Re: [PATCH] HID: amd-sfh: Use shared interrupts

[Basavaraj Natikar <bnatikar@amd.com>]

Hi,


On 10/18/2026 3:34 PM, Ho Jie Feng wrote:
> [You don't often get email from hjf3108@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> The AMD SFH driver requests interrupts without IRQF_SHARED.
> This can cause VFIO PCI passthrough initialization to fail
> when a passed-through device shares the interrupt with AMD MP2.
>
> Request the interrupt with IRQF_SHARED and check if any flags
> are cleared before acknowledging an interrupt.

The IRQF_SHARED conversion and returning IRQ_NONE when no MP2 interrupt
is pending look correct.

Could you please make these updates?

1. This is a long-standing regression introduced by:
Fixes: 7f016b35ca76 ("HID: amd_sfh: Add interrupt handler to process interrupts")
Cc: stable@vger.kernel.org

2. The commit message wording is confusing:
   "check if any flags are cleared before acknowledging an interrupt"

The code checks for a pending MP2 interrupt before clearing/acknowledging
it. Please reword this accordingly.

Also, please include the test environment and the actual failure/success
result: host platform, passed-through PCI device, the original VFIO error,
and confirmation that the device now initializes successfully.

Thanks,
--
Basavaraj

>
> Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
> ---
>   drivers/hid/amd-sfh-hid/amd_sfh_common.h |  6 +++---
>   drivers/hid/amd-sfh-hid/amd_sfh_pcie.c   | 18 ++++++++++++------
>   2 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> index e8cc7f1d289cc..4481a50359de6 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> @@ -85,7 +85,7 @@ struct amd_mp2_ops {
>          void (*stop)(struct amd_mp2_dev *privdata, u16 sensor_idx);
>          void (*stop_all)(struct amd_mp2_dev *privdata);
>          int (*response)(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts);
> -       void (*clear_intr)(struct amd_mp2_dev *privdata);
> +       bool (*clear_intr)(struct amd_mp2_dev *privdata);
>          int (*init_intr)(struct amd_mp2_dev *privdata);
>          int (*discovery_status)(struct amd_mp2_dev *privdata);
>          void (*suspend)(struct amd_mp2_dev *mp2);
> @@ -100,9 +100,9 @@ struct amd_mp2_ops {
>
>   void amd_sfh_work(struct work_struct *work);
>   void amd_sfh_work_buffer(struct work_struct *work);
> -void amd_sfh_clear_intr_v2(struct amd_mp2_dev *privdata);
> +bool amd_sfh_clear_intr_v2(struct amd_mp2_dev *privdata);
>   int amd_sfh_irq_init_v2(struct amd_mp2_dev *privdata);
> -void amd_sfh_clear_intr(struct amd_mp2_dev *privdata);
> +bool amd_sfh_clear_intr(struct amd_mp2_dev *privdata);
>   int amd_sfh_irq_init(struct amd_mp2_dev *privdata);
>
>   static inline u64 amd_get_c2p_val(struct amd_mp2_dev *mp2, u32 idx)
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index b3cb13b8c5e87..f48a096a6d2bc 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> @@ -103,25 +103,31 @@ static void amd_stop_all_sensor_v2(struct amd_mp2_dev *privdata)
>          writel(cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
>   }
>
> -void amd_sfh_clear_intr_v2(struct amd_mp2_dev *privdata)
> +bool amd_sfh_clear_intr_v2(struct amd_mp2_dev *privdata)
>   {
>          if (readl(privdata->mmio + amd_get_p2c_val(privdata, 4))) {
>                  writel(0, privdata->mmio + amd_get_p2c_val(privdata, 4));
>                  writel(0xf, privdata->mmio + amd_get_p2c_val(privdata, 5));
> +               return true;
>          }
> +
> +       return false;
>   }
>
> -void amd_sfh_clear_intr(struct amd_mp2_dev *privdata)
> +bool amd_sfh_clear_intr(struct amd_mp2_dev *privdata)
>   {
>          if (privdata->mp2_ops->clear_intr)
> -               privdata->mp2_ops->clear_intr(privdata);
> +               return privdata->mp2_ops->clear_intr(privdata);
> +
> +       return false;
>   }
>
>   static irqreturn_t amd_sfh_irq_handler(int irq, void *data)
>   {
> -       amd_sfh_clear_intr(data);
> +       if (amd_sfh_clear_intr(data))
> +               return IRQ_HANDLED;
>
> -       return IRQ_HANDLED;
> +       return IRQ_NONE;
>   }
>
>   int amd_sfh_irq_init_v2(struct amd_mp2_dev *privdata)
> @@ -129,7 +135,7 @@ int amd_sfh_irq_init_v2(struct amd_mp2_dev *privdata)
>          pcim_intx(privdata->pdev, true);
>
>          return devm_request_irq(&privdata->pdev->dev, privdata->pdev->irq,
> -                               amd_sfh_irq_handler, 0, DRIVER_NAME, privdata);
> +                               amd_sfh_irq_handler, IRQF_SHARED, DRIVER_NAME, privdata);
>   }
>
>   static int amd_sfh_dis_sts_v2(struct amd_mp2_dev *privdata)
>
> base-commit: ae6927c533a4454570950125ca8ba909c45f20f9
> --
> 2.55.0
>