[PATCH] platform/x86/intel/vsec: reset state before re-enumerating

[PATCH] platform/x86/intel/vsec: reset state before re-enumerating

[Guangshuo Li]

The change referenced by the Fixes tag split the VSEC enumeration logic
out of intel_vsec_pci_probe() and made PCIe error recovery call
intel_vsec_pci_init() directly. This avoids another pcim_enable_device()
devm action and another devm allocation during slot reset, but it also
means that error recovery reuses the priv structure allocated by probe.

On the initial probe, priv is zeroed by devm_kzalloc(), so state[],
suppliers[] and found_caps start from their initial values. During slot
reset, the existing aux devices are removed before re-enumerating, but
the reused priv still records the old devices as registered and the old
capabilities as found.

The next intel_vsec_pci_init() call can therefore short-circuit in
intel_vsec_register_device() with -EEXIST, or stop early because
found_caps already matches info->caps. The aux devices removed during
slot reset are then not recreated, leaving VSEC functionality missing
after PCIe error recovery.

Reset the VSEC registration state after removing the old aux devices and
before re-enumerating them.

Fixes: 348ccc754d89 ("platform/x86/intel/vsec: Fix enable_cnt imbalance on PCIe error recovery")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/platform/x86/intel/vsec.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 3ae4557b32b4..52ac4b4460a2 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -51,6 +51,13 @@ struct vsec_priv {
 	unsigned long found_caps;
 };
 
+static void intel_vsec_reset_state(struct vsec_priv *priv)
+{
+	memset(priv->suppliers, 0, sizeof(priv->suppliers));
+	memset(priv->state, 0, sizeof(priv->state));
+	priv->found_caps = 0;
+}
+
 static const char *intel_vsec_name(enum intel_vsec_id id)
 {
 	switch (id) {
@@ -850,6 +857,7 @@ static pci_ers_result_t intel_vsec_pci_slot_reset(struct pci_dev *pdev)
 {
 	struct intel_vsec_device *intel_vsec_dev;
 	pci_ers_result_t status = PCI_ERS_RESULT_DISCONNECT;
+	struct vsec_priv *priv = pci_get_drvdata(pdev);
 	unsigned long index;
 
 	dev_info(&pdev->dev, "Resetting PCI slot\n");
@@ -870,6 +878,13 @@ static pci_ers_result_t intel_vsec_pci_slot_reset(struct pci_dev *pdev)
 		devm_release_action(&pdev->dev, intel_vsec_remove_aux,
 				    &intel_vsec_dev->auxdev);
 	}
+
+	/*
+	 * intel_vsec_pci_init() reuses priv. Reset the registration state
+	 * after tearing down old aux devices or the rewalk will skip them as
+	 * already registered.
+	 */
+	intel_vsec_reset_state(priv);
 	pci_restore_state(pdev);
 	intel_vsec_pci_init(pdev);
 
-- 
2.43.0

Re: [PATCH] platform/x86/intel/vsec: reset state before re-enumerating

[Lukas Wunner]

On Wed, Jul 08, 2026 at 11:26:47AM +0800, Guangshuo Li wrote:
> +++ b/drivers/platform/x86/intel/vsec.c
> @@ -51,6 +51,13 @@ struct vsec_priv {
>  	unsigned long found_caps;
>  };
>  
> +static void intel_vsec_reset_state(struct vsec_priv *priv)
> +{
> +	memset(priv->suppliers, 0, sizeof(priv->suppliers));
> +	memset(priv->state, 0, sizeof(priv->state));
> +	priv->found_caps = 0;
> +}

Seems fragile to selectively zero only portions of struct vsec_priv.
If the struct is amended later on with additional fields, it's quite
possible that nobody thinks about zeroing them here.

It's probably more robust to zero the entire struct and re-populate
the info field.

I also don't think a separate function to reset the struct is necessary
as I don't expect additional callers down the road.  So I suggest
zeroing the struct inline in intel_vsec_pci_slot_reset().

@David Box:  I've asked off-list for reset recovery to be tested,
I'm hereby renewing that request.  See section "Software error injection"
in: Documentation/PCI/pcieaer-howto.rst

Thanks,

Lukas

Re: [PATCH] platform/x86/intel/vsec: reset state before re-enumerating

[David Box]

On Wed, Jul 08, 2026 at 11:33:07AM +0200, Lukas Wunner wrote:
> On Wed, Jul 08, 2026 at 11:26:47AM +0800, Guangshuo Li wrote:
> > +++ b/drivers/platform/x86/intel/vsec.c
> > @@ -51,6 +51,13 @@ struct vsec_priv {
> >  	unsigned long found_caps;
> >  };
> >  
> > +static void intel_vsec_reset_state(struct vsec_priv *priv)
> > +{
> > +	memset(priv->suppliers, 0, sizeof(priv->suppliers));
> > +	memset(priv->state, 0, sizeof(priv->state));
> > +	priv->found_caps = 0;
> > +}
> 
> Seems fragile to selectively zero only portions of struct vsec_priv.
> If the struct is amended later on with additional fields, it's quite
> possible that nobody thinks about zeroing them here.
> 
> It's probably more robust to zero the entire struct and re-populate
> the info field.
> 
> I also don't think a separate function to reset the struct is necessary
> as I don't expect additional callers down the road.  So I suggest
> zeroing the struct inline in intel_vsec_pci_slot_reset().
> 
> @David Box:  I've asked off-list for reset recovery to be tested,
> I'm hereby renewing that request.  See section "Software error injection"
> in: Documentation/PCI/pcieaer-howto.rst

We'll do the test and let you know. Thanks.

David