From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bernhard Kohl Subject: Re: [PATCH] device-assignment: register a reset function Date: Tue, 16 Nov 2010 12:39:56 +0100 Message-ID: <4CE26D8C.7020804@nsn.com> References: <1284740181.14301.15.camel@x201> <1289820837-24254-1-git-send-email-bernhard.kohl@nsn.com> <4CE122CC.6050700@siemens.com> <1289853510.2805.233.camel@x201> <4CE1ACF3.3020104@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Alex Williamson , Thomas Ostler , kvm , "Michael S. Tsirkin" To: ext Jan Kiszka Return-path: Received: from demumfd002.nsn-inter.net ([93.183.12.31]:11350 "EHLO demumfd002.nsn-inter.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933105Ab0KPLkM (ORCPT ); Tue, 16 Nov 2010 06:40:12 -0500 In-Reply-To: <4CE1ACF3.3020104@web.de> Sender: kvm-owner@vger.kernel.org List-ID: 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 >>>> Signed-off-by: Bernhard Kohl >>>> --- >>>> 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 > >