Re: [PATCH] device-assignment: register a reset function

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 2010-11-15 at 13:08 +0100, Jan Kiszka wrote:
> [Wrong list, it's not upstream yet. I'm migrating the thread to kvm.]
> 
> Am 15.11.2010 12:33, Bernhard Kohl wrote:
> > This is necessary because during reboot of a VM the assigned devices
> > continue DMA transfers which causes memory corruption.
> > 
> > Signed-off-by: Thomas Ostler <thomas.ostler@nsn.com>
> > Signed-off-by: Bernhard Kohl <bernhard.kohl@nsn.com>
> > ---
> > Sorry for for the long delay. Finally we added Alex' suggestions
> > and rebased the patch.
> > 
> > Thanks
> > Bernhard
> > ---
> >  hw/device-assignment.c |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> > index 5f5bde1..3f8de66 100644
> > --- a/hw/device-assignment.c
> > +++ b/hw/device-assignment.c
> > @@ -1434,6 +1434,17 @@ static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
> >      dev->msix_table_page = NULL;
> >  }
> >  
> > +static void reset_assigned_device(DeviceState *dev)
> > +{
> > +    PCIDevice *d = DO_UPCAST(PCIDevice, qdev, dev);
> > +    uint32_t conf;
> > +
> > +    /* reset the bus master bit to avoid further DMA transfers */
> > +    conf = assigned_dev_pci_read_config(d, PCI_COMMAND, 2);
> > +    conf &= ~PCI_COMMAND_MASTER;
> > +    assigned_dev_pci_write_config(d, PCI_COMMAND, conf, 2);
> 
> What about writing to /sys/bus/pci/devices/$DEVICE/reset? You probably
> still need to put the command word into the reset state (ie. no RMW in
> any case, just write 0), but the hardware should receive a reset as well
> - if it is capable of doing a function-level reset, but we should at
> least try.

libvirt doesn't currently give us write access to that file, so it'd
require changes up the stack too.  We could accomplish the same by
deassigning and reassigning the device through KVM, but that seems error
prone.  I'm not entirely convinced it's really necessary to go that far,
I expect there's some physical systems out there that don't reset the
device on a warm reset.  In any case, I think doing this much is at
least a good start.  Thanks,

Alex

> > +}
> > +
> >  static int assigned_initfn(struct PCIDevice *pci_dev)
> >  {
> >      AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
> > @@ -1544,6 +1555,7 @@ static PCIDeviceInfo assign_info = {
> >      .qdev.name    = "pci-assign",
> >      .qdev.desc    = "pass through host pci devices to the guest",
> >      .qdev.size    = sizeof(AssignedDevice),
> > +    .qdev.reset   = reset_assigned_device,
> >      .init         = assigned_initfn,
> >      .exit         = assigned_exitfn,
> >      .config_read  = assigned_dev_pci_read_config,
> 
> Jan
>