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

[Bernhard Kohl <bernhard.kohl@nsn.com>]

Am 15.11.2010 22:58, schrieb ext Jan Kiszka:
> Am 15.11.2010 21:38, Alex Williamson wrote:
>    
>> 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,
>>      
>
> OK, can be done on top of it - but should be done as most systems
> perform a reset that is even stronger than pci_reset_function (I've seen
> devices only recovering after warm reboot).
>
> Still, I would suggest
>
> assigned_dev_pci_write_config(d, PCI_COMMAND, 0, 2);
>
> i.e. reset command word to specified reset state.
>    

Yes, that's reasonable. I will resend the patch after testing.

> Jan
>
>