1. Acrn-Dev
  2. Topics

[RFC PATCH] dm: make reset in passthru_deinit optional

Liu, Yifan1
 

From: Yifan Liu <yifan1.liu@...>

When passing through PCI devices we have an option "no_reset",
which, if specified, does not do device reset before passthrough.
However this option does not take effect when device is deinit-ed.

This patch makes no_reset to be effective in passthru_deinit also.

Signed-off-by: Yifan Liu <yifan1.liu@...>
---
devicemodel/hw/pci/passthrough.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/devicemodel/hw/pci/passthrough.c b/devicemodel/hw/pci/passthrough.c
index 01c2cb7e2..055b2f111 100644
--- a/devicemodel/hw/pci/passthrough.c
+++ b/devicemodel/hw/pci/passthrough.c
@@ -788,6 +788,7 @@ passthru_deinit(struct vmctx *ctx, struct pci_vdev *dev, char *opts)
struct acrn_pcidev pcidev = {};
uint16_t phys_bdf = 0;
char reset_path[60];
+ bool need_reset = true;
int fd;

if (!dev->arg) {
@@ -799,6 +800,8 @@ passthru_deinit(struct vmctx *ctx, struct pci_vdev *dev, char *opts)

ptdev = (struct passthru_dev *) dev->arg;

+ need_reset = ptdev->need_reset;
+
pr_info("vm_reset_ptdev_intx:0x%x-%x, ioapic virpin=%d.\n",
virt_bdf, ptdev->phys_bdf, dev->lintr.ioapic_irq);
if (dev->lintr.pin != 0) {
@@ -824,7 +827,7 @@ passthru_deinit(struct vmctx *ctx, struct pci_vdev *dev, char *opts)
*/
vm_deassign_pcidev(ctx, &pcidev);
}
- if (!is_rtvm && phys_bdf) {
+ if (!is_rtvm && phys_bdf && need_reset) {
memset(reset_path, 0, sizeof(reset_path));
snprintf(reset_path, 40,
"/sys/bus/pci/devices/0000:%02x:%02x.%x/reset",
--
2.25.1

Junjie Mao
 

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On Behalf Of Liu,
Yifan1
Sent: Friday, March 17, 2023 11:19 AM
To: acrn-dev@...
Cc: Liu, Yifan1 <yifan1.liu@...>
Subject: [acrn-dev] [RFC PATCH] dm: make reset in passthru_deinit optional

From: Yifan Liu <yifan1.liu@...>

When passing through PCI devices we have an option "no_reset",
which, if specified, does not do device reset before passthrough.
However this option does not take effect when device is deinit-ed.
I think it's worth describing how we end up having such an option that does only part of the work, in order to show that this patch is fixing something rather than introducing anything new.

BTW, do we have any disclaimer printed when "no_reset" is specified for any device? Considering the implications of that option, we'd better inform users of the consequences in a way that they can always see. Documentation is not a proper option in this case.


This patch makes no_reset to be effective in passthru_deinit also.
You can add here a "Fixes" tag pointing to the commit that added device reset on deinit but did not respect the "no_reset" option.

---
Best Regards
Junjie Mao

Signed-off-by: Yifan Liu <yifan1.liu@...>
---
devicemodel/hw/pci/passthrough.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/devicemodel/hw/pci/passthrough.c b/devicemodel/hw/pci/passthrough.c
index 01c2cb7e2..055b2f111 100644
--- a/devicemodel/hw/pci/passthrough.c
+++ b/devicemodel/hw/pci/passthrough.c
@@ -788,6 +788,7 @@ passthru_deinit(struct vmctx *ctx, struct pci_vdev *dev, char *opts)
struct acrn_pcidev pcidev = {};
uint16_t phys_bdf = 0;
char reset_path[60];
+ bool need_reset = true;
int fd;

if (!dev->arg) {
@@ -799,6 +800,8 @@ passthru_deinit(struct vmctx *ctx, struct pci_vdev *dev, char *opts)

ptdev = (struct passthru_dev *) dev->arg;

+ need_reset = ptdev->need_reset;
+
pr_info("vm_reset_ptdev_intx:0x%x-%x, ioapic virpin=%d.\n",
virt_bdf, ptdev->phys_bdf, dev->lintr.ioapic_irq);
if (dev->lintr.pin != 0) {
@@ -824,7 +827,7 @@ passthru_deinit(struct vmctx *ctx, struct pci_vdev *dev, char *opts)
*/
vm_deassign_pcidev(ctx, &pcidev);
}
- if (!is_rtvm && phys_bdf) {
+ if (!is_rtvm && phys_bdf && need_reset) {
memset(reset_path, 0, sizeof(reset_path));
snprintf(reset_path, 40,
"/sys/bus/pci/devices/0000:%02x:%02x.%x/reset",
--
2.25.1




1 - 2 of 2