On Thu, Apr 17, 2014 at 03:41:56PM +0200, Hannes Reinecke wrote: > On 04/17/2014 02:07 PM, Andreas Färber wrote: > > Am 17.04.2014 08:20, schrieb Hannes Reinecke: > >> On 04/16/2014 07:52 PM, Michael S. Tsirkin wrote: > >>> On Wed, Apr 16, 2014 at 07:47:37PM +0200, Andreas Färber wrote: > >>>> Am 16.04.2014 19:40, schrieb Michael S. Tsirkin: > >>>>> On Wed, Apr 16, 2014 at 06:48:08PM +0200, Andreas Färber wrote: > >>>>>> Am 16.04.2014 18:32, schrieb Alexander Graf: > >>>>>>> > >>>>>>> On 16.04.14 16:44, Hannes Reinecke wrote: > >>>>>>>> MSI-X support has been fixed in qemu, so we can enable it again. > >>>>>>>> > >>>>>>>> Signed-off-by: Hannes Reinecke <h...@suse.de> > >>>>>>>> --- > >>>>>>>> hw/scsi/megasas.c | 19 ++++++------------- > >>>>>>>> 1 file changed, 6 insertions(+), 13 deletions(-) > >>>>>>>> > >>>>>>>> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c > >>>>>>>> index 1781525..df45286 100644 > >>>>>>>> --- a/hw/scsi/megasas.c > >>>>>>>> +++ b/hw/scsi/megasas.c > >>>>>>>> @@ -2084,7 +2084,8 @@ static const VMStateDescription > >>>>>>>> vmstate_megasas = { > >>>>>>>> .minimum_version_id = 0, > >>>>>>>> .minimum_version_id_old = 0, > >>>>>>>> .fields = (VMStateField[]) { > >>>>>>>> - VMSTATE_PCI_DEVICE(parent_obj, MegasasState), > >>>>>>>> + VMSTATE_PCIE_DEVICE(parent_obj, MegasasState), > >>>>>>>> + VMSTATE_MSIX(parent_obj, MegasasState), > >>>>>>> > >>>>>>> This requires a version change for vmstate, no? > >>>>>> > >>>>>> The PCI -> PCIE change yes. mst might have objections for bumping an > >>>>>> x86 > >>>>>> PCI device? > >>>>> > >>>>> Yes. > >>>>> It should be possible to make this conditional on having a pcie > >>>>> capability, and disable pcie capability for old pc versions. > >>>> > >>>> I did have a patch merging their vmstate_ structs based on some test in > >>>> the series I ping'ed, unfortunately the difference was mostly config > >>>> space size, so not immediately obvious how to set via compat_props, but > >>>> likely solvable somehow. > >>> > >>> Check pci_is_express ? > >>> > >>>>>> The MSIX addition should be safe AFAICT since old versions would not > >>>>>> enable MSI-X. > >>>>> > >>>>> Yes but I don't see a code like this - where's code in PC > >>>>> disabling msix for old versions? > >>>> > >>>> The default value for use_msix (use-msix?) in v2 is already false, so no > >>>> need to set it to false again in compat_props? > >>>> > >>>> Andreas > >>> > >>> Ah, I didn't notice that. Sure, if it's off by default there's > >>> no need for compat code. > >>> > >> And the net result is ... what? > >> Do I need to change the code? > > > > Yes, NAK for this patch as is. > > > > Why are you changing VMSTATE_PCI_DEVICE() to VMSTATE_PCIE_DEVICE()? Was > > it always a PCIe device and you're fixing that now? Then put it into its > > own patch with proper explanation, setting pdc->is_express = 1 *and* > > assure there via some compatibility property (?) that migration from > > 2.0.0-rc3 to the patched version succeeds (which would probably still > > break backwards migration that mst was interested in though). > > > The thing was always PCIe; it's just that at the time I've > originally implemented it MSI-X support was broken, and PCIe even > more so. So I figured to take the easy way out and implement it as a > normal PCI device. > > I'm happy to continue with having the original implementation at > PCI, and only moving to PCIe with the gen2 emulation. > > Would that be okay? > > Cheers, > > Hannes
And then gen1 and gen2 get different names? That's okay. Alternatively, I think it's reasonable to add a property which specifies PCI or PCIE. You then force compatible behaviour for old machine types, see e.g. PC_COMPAT_XX macros in include/hw/i386/pc.h > -- > Dr. Hannes Reinecke zSeries & Storage > h...@suse.de +49 911 74053 688 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)