Re: [RFC XEN PATCH v2 1/3] xen/vpci: Clear all vpci status of device

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, Nov 30, 2023 at 06:22:28AM +0000, Chen, Jiqian wrote:
> Hi Roger and Daniel P. Smith,
> 
> On 2023/11/28 22:08, Roger Pau Monné wrote:
> > On Fri, Nov 24, 2023 at 06:41:34PM +0800, Jiqian Chen wrote:
> >> When a device has been reset on dom0 side, the vpci on Xen
> >> side won't get notification, so the cached state in vpci is
> >> all out of date compare with the real device state.
> >> To solve that problem, this patch add new hypercall to clear
> >> all vpci device state. And when reset device happens on dom0
> >> side, dom0 can call this hypercall to notify vpci.
> >>
> >> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
> >> Signed-off-by: Huang Rui <ray.huang@amd.com>
> >> ---
> >>  xen/arch/x86/hvm/hypercall.c  |  1 +
> >>  xen/drivers/passthrough/pci.c | 21 +++++++++++++++++++++
> >>  xen/drivers/pci/physdev.c     | 14 ++++++++++++++
> >>  xen/drivers/vpci/vpci.c       |  9 +++++++++
> >>  xen/include/public/physdev.h  |  2 ++
> >>  xen/include/xen/pci.h         |  1 +
> >>  xen/include/xen/vpci.h        |  6 ++++++
> >>  7 files changed, 54 insertions(+)
> >>
> >> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
> >> index eeb73e1aa5..6ad5b4d5f1 100644
> >> --- a/xen/arch/x86/hvm/hypercall.c
> >> +++ b/xen/arch/x86/hvm/hypercall.c
> >> @@ -84,6 +84,7 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> >>      case PHYSDEVOP_pci_mmcfg_reserved:
> >>      case PHYSDEVOP_pci_device_add:
> >>      case PHYSDEVOP_pci_device_remove:
> >> +    case PHYSDEVOP_pci_device_state_reset:
> >>      case PHYSDEVOP_dbgp_op:
> >>          if ( !is_hardware_domain(currd) )
> >>              return -ENOSYS;
> >> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> >> index 04d00c7c37..f871715585 100644
> >> --- a/xen/drivers/passthrough/pci.c
> >> +++ b/xen/drivers/passthrough/pci.c
> >> @@ -824,6 +824,27 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
> >>      return ret;
> >>  }
> >>  
> >> +int pci_reset_device_state(u16 seg, u8 bus, u8 devfn)
> > 
> > You could use pci_sbdf_t here instead of 3 parameters.
> Will change in next version, thank you.
> 
> > 
> > I'm however unsure whether we really need this helper just to fetch
> > the pdev and call vpci_reset_device_state(), I think you could place
> > this logic directly in pci_physdev_op().  Unless there are plans to
> > use such logic outside of pci_physdev_op().
> If I place the logic of pci_reset_device_state directly in pci_physdev_op. I think I need to declare vpci_reset_device_state in pci.h? If it is suitable, I will change in next version.

Just include xen/vpci.h in drivers/pci/physdev.c AFAICT.

Thanks, Roger.