Re: [Qemu-devel] Re: [kvm-devel] [PATCH 0/3] Qemu crashes with pci passthrough

[Glauber Costa]

On Sat, Apr 19, 2008 at 6:11 PM, Glauber Costa <address@hidden> wrote:
>
> On Fri, Apr 18, 2008 at 1:27 PM, Avi Kivity <address@hidden> wrote:
>  >
>  > Glauber de Oliveira Costa wrote:
>  >
>  > > Hi,
>  > > I've got some qemu crashes while trying to passthrough an ide device
>  > > to a kvm guest. After some investigation, it turned out that
>  > register_ioport_{read/write} will abort on errors instead of returning
>  > > a meaningful error.
>  > >
>  > > However, even if we do return an error, the asynchronous nature of pci
>  > > config space mapping updates makes it a little bit hard to treat.
>  > >
>  > > This series of patches basically treats errors in the mapping functions 
> in
>  > > the pci layer. If anything goes wrong, we unregister the pci device,
>  > unmapping
>  > > any mappings that happened to be sucessfull already.
>  > >
>  > > After these patches are applied, a lot of warnings appears. And, you 
> know,
>  > > everytime there is a warning, god kills a kitten. But I'm not planning on
>  > > touching the other pieces of qemu code for this until we set up (or not)
>  > in
>  > > this solution
>  > >
>  > > Comments are very welcome, specially from qemu folks (since it is a bit
>  > invasive)
>  > >
>  > >
>  > >
>  >
>  >  Have you considered, instead of rolling back the changes you already made
>  > before the failure, to have a function which checks if an ioport
>  > registration will be successful?  This may simplify the code.
>  >
>  Yes, I did.
>
>  Basic problem is that I basically could not find this information
>  handy until we were deep in the stack, right before calling the update
>  mapping functions. I turned out preferring this option. I can,
>  however, take a fresh look at that.
>

Looked at this again, and it does seem to me that we don't have too
much to gain from a "test-before" solution. We definitely can't test
it reliably until update_mappings arrive, (since the mapping can
change) and by this time, the pci device is already registered, and we
would have to de-register it anyway. There is room for "improvement"
(with a wide definition of improvement) if we test all the ports of a
device in advance (inside update_mappings) instead of a port-by-port
basis. We could get rid of the flag, but it would be traded off by
another complexities.

So unless someone have a very direct alternate solution for this I'm
failing to see, I do advocate for those humble patches.

-- 
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."