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). Anyway, this MSI-X patch adding only VMSTATE_MSIX() after VMSTATE_PCI[E]_DEVICE() then becomes trivially correct IMO. If you wanted to turn on MSI-X by default as in v1, then you would need a trivial three-line struct entry in include/hw/i386/pc.h:PC_COMPAT_2_0 setting the property to the old default of false. Cheers, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg