All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 0/4] qdev: switch reset to post-order, clean up PCI reset
Date: Wed, 09 Oct 2013 18:10:49 +0200	[thread overview]
Message-ID: <52558009.4000701@redhat.com> (raw)
In-Reply-To: <5251C96D.1020709@redhat.com>

Il 06/10/2013 22:34, Paolo Bonzini ha scritto:
> Il 06/10/2013 20:28, Michael S. Tsirkin ha scritto:
>>>>> For each PCI device I tried creating a VM with an instance of it (a few
>>>>> devices at a time), and did VM resets.  Earlier versions were tested by
>>>>> the guy who reported the SCSI problems.
>>>>
>>>> x86 kvm only?
>>>
>>> Yes.
>>
>> Hmm, I'm not sure that's enough for this kind of change.
> 
> I'll do more tests though, from looking at the source code, I'm not sure
> what could happen depending on the host bridge.

Did more tests, PPC g3beige and PPC64 mac99 both work.

I also tested resetting the secondary bus of a PCI bridge (via setpci),
and it also works as expected.

Finally, I looked more at the history of the code to justify patch 2.

Initially, zeroing the irq_state was added in commit 6eaa684 (Add
pci_bus_reset() function., 2009-06-17) to deal with this issue:

>> Shouldn't each device's reset function bring its line low, thus zeroing  
>> the irq_state naturally?
>>
>> If not, we have a bug somewhere.  Note we have exactly the same issue  
>> with save/restore.
>>
> They should, but I haven't found one that does.

More registers were then cleared by pci_device_reset in your commit
c0b1905 (qemu/pci: reset device registers on bus reset, 2009-09-16).
Deasserting interrupts explicitly came in later as part of PCI bus and
FLR support in commit 4c92325 (pci: deassert intx on reset.,
2011-01-20).  That should be the point where the code starts following
the invariant of patch 2.

Paolo

  reply	other threads:[~2013-10-09 16:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-03 13:46 [Qemu-devel] [PATCH 0/4] qdev: switch reset to post-order, clean up PCI reset Paolo Bonzini
2013-10-03 13:46 ` [Qemu-devel] [PATCH 1/4] pci: do not export pci_bus_reset Paolo Bonzini
2013-10-03 13:46 ` [Qemu-devel] [PATCH 2/4] pci: clean up resetting of IRQs Paolo Bonzini
2013-10-03 13:46 ` [Qemu-devel] [PATCH 3/4] qdev: allow both pre- and post-order vists in qdev walking functions Paolo Bonzini
2013-10-03 13:46 ` [Qemu-devel] [PATCH 4/4] qdev: switch reset to post-order Paolo Bonzini
2013-10-03 13:54 ` [Qemu-devel] [PATCH 0/4] qdev: switch reset to post-order, clean up PCI reset Michael S. Tsirkin
2013-10-03 15:58   ` Paolo Bonzini
2013-10-03 16:54     ` Michael S. Tsirkin
2013-10-03 16:53       ` Paolo Bonzini
2013-10-06 18:28         ` Michael S. Tsirkin
2013-10-06 20:34           ` Paolo Bonzini
2013-10-09 16:10             ` Paolo Bonzini [this message]
  -- strict thread matches above, loose matches on Subject: below --
2013-12-06 16:54 Paolo Bonzini
2013-12-19 19:15 ` Michael S. Tsirkin
2013-12-19 23:45   ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52558009.4000701@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.