| Message ID | 20230503181240.14009-11-shannon.nelson@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pds_vdpa driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, May 4, 2023 at 2:13 AM Shannon Nelson <shannon.nelson@amd.com> wrote: > > Register for the pds_core's notification events, primarily to > find out when the FW has been reset so we can pass this on > back up the chain. > > Signed-off-by: Shannon Nelson <shannon.nelson@amd.com> > --- > drivers/vdpa/pds/vdpa_dev.c | 68 ++++++++++++++++++++++++++++++++++++- > drivers/vdpa/pds/vdpa_dev.h | 1 + > 2 files changed, 68 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c > index 9970657cdb3d..377eefc2fa1e 100644 > --- a/drivers/vdpa/pds/vdpa_dev.c > +++ b/drivers/vdpa/pds/vdpa_dev.c > @@ -21,6 +21,61 @@ static struct pds_vdpa_device *vdpa_to_pdsv(struct vdpa_device *vdpa_dev) > return container_of(vdpa_dev, struct pds_vdpa_device, vdpa_dev); > } > > +static int pds_vdpa_notify_handler(struct notifier_block *nb, > + unsigned long ecode, > + void *data) > +{ > + struct pds_vdpa_device *pdsv = container_of(nb, struct pds_vdpa_device, nb); > + struct device *dev = &pdsv->vdpa_aux->padev->aux_dev.dev; > + > + dev_dbg(dev, "%s: event code %lu\n", __func__, ecode); > + > + /* Give the upper layers a hint that something interesting > + * may have happened. It seems that the only thing this > + * triggers in the virtio-net drivers above us is a check > + * of link status. > + * > + * We don't set the NEEDS_RESET flag for EVENT_RESET > + * because we're likely going through a recovery or > + * fw_update and will be back up and running soon. > + */ > + if (ecode == PDS_EVENT_RESET || ecode == PDS_EVENT_LINK_CHANGE) { The code here seems to conflict with the comment above. If we don't set NEEDS_RESET, there's no need for the config callback? Thanks > + if (pdsv->config_cb.callback) > + pdsv->config_cb.callback(pdsv->config_cb.private); > + } > + > + return 0; > +} > + > +static int pds_vdpa_register_event_handler(struct pds_vdpa_device *pdsv) > +{ > + struct device *dev = &pdsv->vdpa_aux->padev->aux_dev.dev; > + struct notifier_block *nb = &pdsv->nb; > + int err; > + > + if (!nb->notifier_call) { > + nb->notifier_call = pds_vdpa_notify_handler; > + err = pdsc_register_notify(nb); > + if (err) { > + nb->notifier_call = NULL; > + dev_err(dev, "failed to register pds event handler: %ps\n", > + ERR_PTR(err)); > + return -EINVAL; > + } > + dev_dbg(dev, "pds event handler registered\n"); > + } > + > + return 0; > +} > + > +static void pds_vdpa_unregister_event_handler(struct pds_vdpa_device *pdsv) > +{ > + if (pdsv->nb.notifier_call) { > + pdsc_unregister_notify(&pdsv->nb); > + pdsv->nb.notifier_call = NULL; > + } > +} > + > static int pds_vdpa_set_vq_address(struct vdpa_device *vdpa_dev, u16 qid, > u64 desc_addr, u64 driver_addr, u64 device_addr) > { > @@ -522,6 +577,12 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, > > pdsv->vdpa_dev.mdev = &vdpa_aux->vdpa_mdev; > > + err = pds_vdpa_register_event_handler(pdsv); > + if (err) { > + dev_err(dev, "Failed to register for PDS events: %pe\n", ERR_PTR(err)); > + goto err_unmap; > + } > + > /* We use the _vdpa_register_device() call rather than the > * vdpa_register_device() to avoid a deadlock because our > * dev_add() is called with the vdpa_dev_lock already set > @@ -530,13 +591,15 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, > err = _vdpa_register_device(&pdsv->vdpa_dev, pdsv->num_vqs); > if (err) { > dev_err(dev, "Failed to register to vDPA bus: %pe\n", ERR_PTR(err)); > - goto err_unmap; > + goto err_unevent; > } > > pds_vdpa_debugfs_add_vdpadev(vdpa_aux); > > return 0; > > +err_unevent: > + pds_vdpa_unregister_event_handler(pdsv); > err_unmap: > put_device(&pdsv->vdpa_dev.dev); > vdpa_aux->pdsv = NULL; > @@ -546,8 +609,11 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, > static void pds_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, > struct vdpa_device *vdpa_dev) > { > + struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev); > struct pds_vdpa_aux *vdpa_aux; > > + pds_vdpa_unregister_event_handler(pdsv); > + > vdpa_aux = container_of(mdev, struct pds_vdpa_aux, vdpa_mdev); > _vdpa_unregister_device(vdpa_dev); > > diff --git a/drivers/vdpa/pds/vdpa_dev.h b/drivers/vdpa/pds/vdpa_dev.h > index a21596f438c1..1650a2b08845 100644 > --- a/drivers/vdpa/pds/vdpa_dev.h > +++ b/drivers/vdpa/pds/vdpa_dev.h > @@ -40,6 +40,7 @@ struct pds_vdpa_device { > u8 vdpa_index; /* rsvd for future subdevice use */ > u8 num_vqs; /* num vqs in use */ > struct vdpa_callback config_cb; > + struct notifier_block nb; > }; > > int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux); > -- > 2.17.1 >
On 5/14/23 10:02 PM, Jason Wang wrote: > On Thu, May 4, 2023 at 2:13 AM Shannon Nelson <shannon.nelson@amd.com> wrote: >> >> Register for the pds_core's notification events, primarily to >> find out when the FW has been reset so we can pass this on >> back up the chain. >> >> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com> >> --- >> drivers/vdpa/pds/vdpa_dev.c | 68 ++++++++++++++++++++++++++++++++++++- >> drivers/vdpa/pds/vdpa_dev.h | 1 + >> 2 files changed, 68 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c >> index 9970657cdb3d..377eefc2fa1e 100644 >> --- a/drivers/vdpa/pds/vdpa_dev.c >> +++ b/drivers/vdpa/pds/vdpa_dev.c >> @@ -21,6 +21,61 @@ static struct pds_vdpa_device *vdpa_to_pdsv(struct vdpa_device *vdpa_dev) >> return container_of(vdpa_dev, struct pds_vdpa_device, vdpa_dev); >> } >> >> +static int pds_vdpa_notify_handler(struct notifier_block *nb, >> + unsigned long ecode, >> + void *data) >> +{ >> + struct pds_vdpa_device *pdsv = container_of(nb, struct pds_vdpa_device, nb); >> + struct device *dev = &pdsv->vdpa_aux->padev->aux_dev.dev; >> + >> + dev_dbg(dev, "%s: event code %lu\n", __func__, ecode); >> + >> + /* Give the upper layers a hint that something interesting >> + * may have happened. It seems that the only thing this >> + * triggers in the virtio-net drivers above us is a check >> + * of link status. >> + * >> + * We don't set the NEEDS_RESET flag for EVENT_RESET >> + * because we're likely going through a recovery or >> + * fw_update and will be back up and running soon. >> + */ >> + if (ecode == PDS_EVENT_RESET || ecode == PDS_EVENT_LINK_CHANGE) { > > The code here seems to conflict with the comment above. If we don't > set NEEDS_RESET, there's no need for the config callback? Yes, that's an out-of-date comment that should be removed. I think we really just need to pass up the stack the hint that something interesting may have happened and let the upper layers decide what they want to do with whatever info they have available. I'll clean up this comment block. sln > > Thanks > >> + if (pdsv->config_cb.callback) >> + pdsv->config_cb.callback(pdsv->config_cb.private); >> + } >> + >> + return 0; >> +} >> + >> +static int pds_vdpa_register_event_handler(struct pds_vdpa_device *pdsv) >> +{ >> + struct device *dev = &pdsv->vdpa_aux->padev->aux_dev.dev; >> + struct notifier_block *nb = &pdsv->nb; >> + int err; >> + >> + if (!nb->notifier_call) { >> + nb->notifier_call = pds_vdpa_notify_handler; >> + err = pdsc_register_notify(nb); >> + if (err) { >> + nb->notifier_call = NULL; >> + dev_err(dev, "failed to register pds event handler: %ps\n", >> + ERR_PTR(err)); >> + return -EINVAL; >> + } >> + dev_dbg(dev, "pds event handler registered\n"); >> + } >> + >> + return 0; >> +} >> + >> +static void pds_vdpa_unregister_event_handler(struct pds_vdpa_device *pdsv) >> +{ >> + if (pdsv->nb.notifier_call) { >> + pdsc_unregister_notify(&pdsv->nb); >> + pdsv->nb.notifier_call = NULL; >> + } >> +} >> + >> static int pds_vdpa_set_vq_address(struct vdpa_device *vdpa_dev, u16 qid, >> u64 desc_addr, u64 driver_addr, u64 device_addr) >> { >> @@ -522,6 +577,12 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, >> >> pdsv->vdpa_dev.mdev = &vdpa_aux->vdpa_mdev; >> >> + err = pds_vdpa_register_event_handler(pdsv); >> + if (err) { >> + dev_err(dev, "Failed to register for PDS events: %pe\n", ERR_PTR(err)); >> + goto err_unmap; >> + } >> + >> /* We use the _vdpa_register_device() call rather than the >> * vdpa_register_device() to avoid a deadlock because our >> * dev_add() is called with the vdpa_dev_lock already set >> @@ -530,13 +591,15 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, >> err = _vdpa_register_device(&pdsv->vdpa_dev, pdsv->num_vqs); >> if (err) { >> dev_err(dev, "Failed to register to vDPA bus: %pe\n", ERR_PTR(err)); >> - goto err_unmap; >> + goto err_unevent; >> } >> >> pds_vdpa_debugfs_add_vdpadev(vdpa_aux); >> >> return 0; >> >> +err_unevent: >> + pds_vdpa_unregister_event_handler(pdsv); >> err_unmap: >> put_device(&pdsv->vdpa_dev.dev); >> vdpa_aux->pdsv = NULL; >> @@ -546,8 +609,11 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, >> static void pds_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, >> struct vdpa_device *vdpa_dev) >> { >> + struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev); >> struct pds_vdpa_aux *vdpa_aux; >> >> + pds_vdpa_unregister_event_handler(pdsv); >> + >> vdpa_aux = container_of(mdev, struct pds_vdpa_aux, vdpa_mdev); >> _vdpa_unregister_device(vdpa_dev); >> >> diff --git a/drivers/vdpa/pds/vdpa_dev.h b/drivers/vdpa/pds/vdpa_dev.h >> index a21596f438c1..1650a2b08845 100644 >> --- a/drivers/vdpa/pds/vdpa_dev.h >> +++ b/drivers/vdpa/pds/vdpa_dev.h >> @@ -40,6 +40,7 @@ struct pds_vdpa_device { >> u8 vdpa_index; /* rsvd for future subdevice use */ >> u8 num_vqs; /* num vqs in use */ >> struct vdpa_callback config_cb; >> + struct notifier_block nb; >> }; >> >> int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux); >> -- >> 2.17.1 >> >
diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c index 9970657cdb3d..377eefc2fa1e 100644 --- a/drivers/vdpa/pds/vdpa_dev.c +++ b/drivers/vdpa/pds/vdpa_dev.c @@ -21,6 +21,61 @@ static struct pds_vdpa_device *vdpa_to_pdsv(struct vdpa_device *vdpa_dev) return container_of(vdpa_dev, struct pds_vdpa_device, vdpa_dev); } +static int pds_vdpa_notify_handler(struct notifier_block *nb, + unsigned long ecode, + void *data) +{ + struct pds_vdpa_device *pdsv = container_of(nb, struct pds_vdpa_device, nb); + struct device *dev = &pdsv->vdpa_aux->padev->aux_dev.dev; + + dev_dbg(dev, "%s: event code %lu\n", __func__, ecode); + + /* Give the upper layers a hint that something interesting + * may have happened. It seems that the only thing this + * triggers in the virtio-net drivers above us is a check + * of link status. + * + * We don't set the NEEDS_RESET flag for EVENT_RESET + * because we're likely going through a recovery or + * fw_update and will be back up and running soon. + */ + if (ecode == PDS_EVENT_RESET || ecode == PDS_EVENT_LINK_CHANGE) { + if (pdsv->config_cb.callback) + pdsv->config_cb.callback(pdsv->config_cb.private); + } + + return 0; +} + +static int pds_vdpa_register_event_handler(struct pds_vdpa_device *pdsv) +{ + struct device *dev = &pdsv->vdpa_aux->padev->aux_dev.dev; + struct notifier_block *nb = &pdsv->nb; + int err; + + if (!nb->notifier_call) { + nb->notifier_call = pds_vdpa_notify_handler; + err = pdsc_register_notify(nb); + if (err) { + nb->notifier_call = NULL; + dev_err(dev, "failed to register pds event handler: %ps\n", + ERR_PTR(err)); + return -EINVAL; + } + dev_dbg(dev, "pds event handler registered\n"); + } + + return 0; +} + +static void pds_vdpa_unregister_event_handler(struct pds_vdpa_device *pdsv) +{ + if (pdsv->nb.notifier_call) { + pdsc_unregister_notify(&pdsv->nb); + pdsv->nb.notifier_call = NULL; + } +} + static int pds_vdpa_set_vq_address(struct vdpa_device *vdpa_dev, u16 qid, u64 desc_addr, u64 driver_addr, u64 device_addr) { @@ -522,6 +577,12 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, pdsv->vdpa_dev.mdev = &vdpa_aux->vdpa_mdev; + err = pds_vdpa_register_event_handler(pdsv); + if (err) { + dev_err(dev, "Failed to register for PDS events: %pe\n", ERR_PTR(err)); + goto err_unmap; + } + /* We use the _vdpa_register_device() call rather than the * vdpa_register_device() to avoid a deadlock because our * dev_add() is called with the vdpa_dev_lock already set @@ -530,13 +591,15 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, err = _vdpa_register_device(&pdsv->vdpa_dev, pdsv->num_vqs); if (err) { dev_err(dev, "Failed to register to vDPA bus: %pe\n", ERR_PTR(err)); - goto err_unmap; + goto err_unevent; } pds_vdpa_debugfs_add_vdpadev(vdpa_aux); return 0; +err_unevent: + pds_vdpa_unregister_event_handler(pdsv); err_unmap: put_device(&pdsv->vdpa_dev.dev); vdpa_aux->pdsv = NULL; @@ -546,8 +609,11 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, static void pds_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *vdpa_dev) { + struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev); struct pds_vdpa_aux *vdpa_aux; + pds_vdpa_unregister_event_handler(pdsv); + vdpa_aux = container_of(mdev, struct pds_vdpa_aux, vdpa_mdev); _vdpa_unregister_device(vdpa_dev); diff --git a/drivers/vdpa/pds/vdpa_dev.h b/drivers/vdpa/pds/vdpa_dev.h index a21596f438c1..1650a2b08845 100644 --- a/drivers/vdpa/pds/vdpa_dev.h +++ b/drivers/vdpa/pds/vdpa_dev.h @@ -40,6 +40,7 @@ struct pds_vdpa_device { u8 vdpa_index; /* rsvd for future subdevice use */ u8 num_vqs; /* num vqs in use */ struct vdpa_callback config_cb; + struct notifier_block nb; }; int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux);
Register for the pds_core's notification events, primarily to find out when the FW has been reset so we can pass this on back up the chain. Signed-off-by: Shannon Nelson <shannon.nelson@amd.com> --- drivers/vdpa/pds/vdpa_dev.c | 68 ++++++++++++++++++++++++++++++++++++- drivers/vdpa/pds/vdpa_dev.h | 1 + 2 files changed, 68 insertions(+), 1 deletion(-)